Browse Source

Fixed #25232 -- Made ModelBackend/RemoteUserBackend reject inactive users.

Alexander Gaevsky 9 years ago
parent
commit
e0a3d93730

+ 24 - 4
django/contrib/auth/backends.py

@@ -15,12 +15,21 @@ class ModelBackend(object):
             username = kwargs.get(UserModel.USERNAME_FIELD)
         try:
             user = UserModel._default_manager.get_by_natural_key(username)
-            if user.check_password(password):
-                return user
         except UserModel.DoesNotExist:
             # Run the default password hasher once to reduce the timing
             # difference between an existing and a non-existing user (#20760).
             UserModel().set_password(password)
+        else:
+            if user.check_password(password) and self.user_can_authenticate(user):
+                return user
+
+    def user_can_authenticate(self, user):
+        """
+        Reject users with is_active=False. Custom user models that don't have
+        that attribute are allowed.
+        """
+        is_active = getattr(user, 'is_active', None)
+        return is_active or is_active is None
 
     def _get_user_permissions(self, user_obj):
         return user_obj.user_permissions.all()
@@ -90,9 +99,15 @@ class ModelBackend(object):
     def get_user(self, user_id):
         UserModel = get_user_model()
         try:
-            return UserModel._default_manager.get(pk=user_id)
+            user = UserModel._default_manager.get(pk=user_id)
         except UserModel.DoesNotExist:
             return None
+        return user if self.user_can_authenticate(user) else None
+
+
+class AllowAllUsersModelBackend(ModelBackend):
+    def user_can_authenticate(self, user):
+        return True
 
 
 class RemoteUserBackend(ModelBackend):
@@ -140,7 +155,7 @@ class RemoteUserBackend(ModelBackend):
                 user = UserModel._default_manager.get_by_natural_key(username)
             except UserModel.DoesNotExist:
                 pass
-        return user
+        return user if self.user_can_authenticate(user) else None
 
     def clean_username(self, username):
         """
@@ -158,3 +173,8 @@ class RemoteUserBackend(ModelBackend):
         By default, returns the user unmodified.
         """
         return user
+
+
+class AllowAllUsersRemoteUserBackend(RemoteUserBackend):
+    def user_can_authenticate(self, user):
+        return True

+ 14 - 3
docs/howto/auth-remote-user.txt

@@ -64,9 +64,20 @@ remote users. These interfaces work with users stored in the database
 regardless of ``AUTHENTICATION_BACKENDS``.
 
 .. note::
-   Since the ``RemoteUserBackend`` inherits from ``ModelBackend``, you will
-   still have all of the same permissions checking that is implemented in
-   ``ModelBackend``.
+
+    Since the ``RemoteUserBackend`` inherits from ``ModelBackend``, you will
+    still have all of the same permissions checking that is implemented in
+    ``ModelBackend``.
+
+    Users with :attr:`is_active=False
+    <django.contrib.auth.models.User.is_active>` won't be allowed to
+    authenticate. Use
+    :class:`~django.contrib.auth.backends.AllowAllUsersRemoteUserBackend` if
+    you want to allow them to.
+
+    .. versionchanged:: 1.10
+
+        In older versions, inactive users weren't rejected as described above.
 
 If your authentication mechanism uses a custom HTTP header and not
 ``REMOTE_USER``, you can subclass ``RemoteUserMiddleware`` and set the

+ 63 - 8
docs/ref/contrib/auth.txt

@@ -76,15 +76,26 @@ Fields
 
         This doesn't necessarily control whether or not the user can log in.
         Authentication backends aren't required to check for the ``is_active``
-        flag, and the default backends do not. If you want to reject a login
-        based on ``is_active`` being ``False``, it's up to you to check that in
-        your own login view or a custom authentication backend. However, the
+        flag but the default backend
+        (:class:`~django.contrib.auth.backends.ModelBackend`) and the
+        :class:`~django.contrib.auth.backends.RemoteUserBackend` do. You can
+        use :class:`~django.contrib.auth.backends.AllowAllUsersModelBackend`
+        or :class:`~django.contrib.auth.backends.AllowAllUsersRemoteUserBackend`
+        if you want to allow inactive users to login. In this case, you'll also
+        want to customize the
         :class:`~django.contrib.auth.forms.AuthenticationForm` used by the
-        :func:`~django.contrib.auth.views.login` view (which is the default)
-        *does* perform this check, as do the permission-checking methods such
-        as :meth:`~django.contrib.auth.models.User.has_perm` and the
-        authentication in the Django admin. All of those functions/methods will
-        return ``False`` for inactive users.
+        :func:`~django.contrib.auth.views.login` view as it rejects inactive
+        users. Be aware that the permission-checking methods such as
+        :meth:`~django.contrib.auth.models.User.has_perm` and the
+        authentication in the Django admin all return ``False`` for inactive
+        users.
+
+        .. versionchanged:: 1.10
+
+            In older versions,
+            :class:`~django.contrib.auth.backends.ModelBackend` and
+            :class:`~django.contrib.auth.backends.RemoteUserBackend` allowed
+            inactive users to authenticate.
 
     .. attribute:: is_superuser
 
@@ -488,6 +499,32 @@ The following backends are available in :mod:`django.contrib.auth.backends`:
         Returns whether the ``user_obj`` has any permissions on the app
         ``app_label``.
 
+    .. method:: ModelBackend.user_can_authenticate()
+
+        .. versionadded:: 1.10
+
+        Returns whether the user is allowed to authenticate. To match the
+        behavior of :class:`~django.contrib.auth.forms.AuthenticationForm`
+        which :meth:`prohibits inactive users from logging in
+        <django.contrib.auth.forms.AuthenticationForm.confirm_login_allowed>`,
+        this method returns ``False`` for users with :attr:`is_active=False
+        <django.contrib.auth.models.User.is_active>`. Custom user models that
+        don't have an :attr:`~django.contrib.auth.models.CustomUser.is_active`
+        field are allowed.
+
+.. class:: AllowAllUsersModelBackend
+
+   .. versionadded:: 1.10
+
+   Same as :class:`ModelBackend` except that it doesn't reject inactive users
+   because :meth:`~ModelBackend.user_can_authenticate` always returns ``True``.
+
+   When using this backend, you'll likely want to customize the
+   :class:`~django.contrib.auth.forms.AuthenticationForm` used by the
+   :func:`~django.contrib.auth.views.login` view by overriding the
+   :meth:`~django.contrib.auth.forms.AuthenticationForm.confirm_login_allowed`
+   method as it rejects inactive users.
+
 .. class:: RemoteUserBackend
 
     Use this backend to take advantage of external-to-Django-handled
@@ -529,3 +566,21 @@ The following backends are available in :mod:`django.contrib.auth.backends`:
    new user is created, and can be used to perform custom setup actions, such
    as setting the user's groups based on attributes in an LDAP directory.
    Returns the user object.
+
+.. method:: RemoteUserBackend.user_can_authenticate()
+
+    .. versionadded:: 1.10
+
+    Returns whether the user is allowed to authenticate. This method returns
+    ``False`` for users with :attr:`is_active=False
+    <django.contrib.auth.models.User.is_active>`. Custom user models that don't
+    have an :attr:`~django.contrib.auth.models.CustomUser.is_active` field are
+    allowed.
+
+.. class:: AllowAllUsersRemoteUserBackend
+
+   .. versionadded:: 1.10
+
+   Same as :class:`RemoteUserBackend` except that it doesn't reject inactive
+   users because :attr:`~RemoteUserBackend.user_can_authenticate` always
+   returns ``True``.

+ 9 - 0
docs/releases/1.10.txt

@@ -669,6 +669,15 @@ Miscellaneous
   calling ``Command.execute()``, pass the command object as the first argument
   to ``call_command()``.
 
+* :class:`~django.contrib.auth.backends.ModelBackend` and
+  :class:`~django.contrib.auth.backends.RemoteUserBackend` now reject inactive
+  users. This means that inactive users can't login and will be logged
+  out if they are switched from ``is_active=True`` to ``False``. If you need
+  the previous behavior, use the new
+  :class:`~django.contrib.auth.backends.AllowAllUsersModelBackend` or
+  :class:`~django.contrib.auth.backends.AllowAllUsersRemoteUserBackend`
+  in :setting:`AUTHENTICATION_BACKENDS` instead.
+
 .. _deprecated-features-1.10:
 
 Features deprecated in 1.10

+ 15 - 4
docs/topics/auth/customizing.txt

@@ -235,10 +235,17 @@ for example, to control anonymous access.
 Authorization for inactive users
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-An inactive user is a one that is authenticated but has its attribute
-``is_active`` set to ``False``. However this does not mean they are not
-authorized to do anything. For example they are allowed to activate their
-account.
+An inactive user is a one that has its
+:attr:`~django.contrib.auth.models.User.is_active` field set to ``False``. The
+:class:`~django.contrib.auth.backends.ModelBackend` and
+:class:`~django.contrib.auth.backends.RemoteUserBackend` authentication
+backends prohibits these users from authenticating. If a custom user model
+doesn't have an :attr:`~django.contrib.auth.models.CustomUser.is_active` field,
+all users will be allowed to authenticate.
+
+You can use :class:`~django.contrib.auth.backends.AllowAllUsersModelBackend`
+or :class:`~django.contrib.auth.backends.AllowAllUsersRemoteUserBackend` if you
+want to allow inactive users to authenticate.
 
 The support for anonymous users in the permission system allows for a scenario
 where anonymous users have permissions to do something while inactive
@@ -247,6 +254,10 @@ authenticated users do not.
 Do not forget to test for the ``is_active`` attribute of the user in your own
 backend permission methods.
 
+.. versionchanged:: 1.10
+
+    In older versions, the :class:`~django.contrib.auth.backends.ModelBackend`
+    allowed inactive users to authenticate.
 
 Handling object permissions
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~

+ 6 - 4
tests/auth_tests/models/__init__.py

@@ -1,12 +1,14 @@
 from .custom_permissions import CustomPermissionsUser
-from .custom_user import CustomUser, ExtensionUser
+from .custom_user import (
+    CustomUser, CustomUserWithoutIsActiveField, ExtensionUser,
+)
 from .invalid_models import CustomUserNonUniqueUsername
 from .is_active import IsActiveTestUser1
 from .uuid_pk import UUIDUser
 from .with_foreign_key import CustomUserWithFK, Email
 
 __all__ = (
-    'CustomUser', 'CustomPermissionsUser', 'CustomUserWithFK', 'Email',
-    'ExtensionUser', 'IsActiveTestUser1', 'UUIDUser',
-    'CustomUserNonUniqueUsername',
+    'CustomUser', 'CustomUserWithoutIsActiveField', 'CustomPermissionsUser',
+    'CustomUserWithFK', 'Email', 'ExtensionUser', 'IsActiveTestUser1',
+    'UUIDUser', 'CustomUserNonUniqueUsername',
 )

+ 9 - 0
tests/auth_tests/models/custom_user.py

@@ -97,6 +97,15 @@ class RemoveGroupsAndPermissions(object):
         PermissionsMixin._meta.local_many_to_many = self._old_pm_local_m2m
 
 
+class CustomUserWithoutIsActiveField(AbstractBaseUser):
+    username = models.CharField(max_length=150, unique=True)
+    email = models.EmailField(unique=True)
+
+    objects = UserManager()
+
+    USERNAME_FIELD = 'username'
+
+
 # The extension user is a simple extension of the built-in user class,
 # adding a required date_of_birth field. This allows us to check for
 # any hard references to the name "User" in forms/handlers etc.

+ 51 - 6
tests/auth_tests/test_auth_backends.py

@@ -15,7 +15,10 @@ from django.test import (
     SimpleTestCase, TestCase, modify_settings, override_settings,
 )
 
-from .models import CustomPermissionsUser, CustomUser, ExtensionUser, UUIDUser
+from .models import (
+    CustomPermissionsUser, CustomUser, CustomUserWithoutIsActiveField,
+    ExtensionUser, UUIDUser,
+)
 
 
 class CountingMD5PasswordHasher(MD5PasswordHasher):
@@ -200,19 +203,35 @@ class ModelBackendTest(BaseModelBackendTest, TestCase):
     Tests for the ModelBackend using the default User model.
     """
     UserModel = User
+    user_credentials = {'username': 'test', 'password': 'test'}
 
     def create_users(self):
-        self.user = User.objects.create_user(
-            username='test',
-            email='test@example.com',
-            password='test',
-        )
+        self.user = User.objects.create_user(email='test@example.com', **self.user_credentials)
         self.superuser = User.objects.create_superuser(
             username='test2',
             email='test2@example.com',
             password='test',
         )
 
+    def test_authenticate_inactive(self):
+        """
+        An inactive user can't authenticate.
+        """
+        self.assertEqual(authenticate(**self.user_credentials), self.user)
+        self.user.is_active = False
+        self.user.save()
+        self.assertIsNone(authenticate(**self.user_credentials))
+
+    @override_settings(AUTH_USER_MODEL='auth_tests.CustomUserWithoutIsActiveField')
+    def test_authenticate_user_without_is_active_field(self):
+        """
+        A custom user without an `is_active` field is allowed to authenticate.
+        """
+        user = CustomUserWithoutIsActiveField.objects._create_user(
+            username='test', email='test@example.com', password='test',
+        )
+        self.assertEqual(authenticate(username='test', password='test'), user)
+
 
 @override_settings(AUTH_USER_MODEL='auth_tests.ExtensionUser')
 class ExtensionUserModelBackendTest(BaseModelBackendTest, TestCase):
@@ -676,3 +695,29 @@ class SelectingBackendTests(TestCase):
         user = User.objects.create_user(self.username, 'email', self.password)
         self.client._login(user, self.other_backend)
         self.assertBackendInSession(self.other_backend)
+
+
+@override_settings(AUTHENTICATION_BACKENDS=['django.contrib.auth.backends.AllowAllUsersModelBackend'])
+class AllowAllUsersModelBackendTest(TestCase):
+    """
+    Inactive users may authenticate with the AllowAllUsersModelBackend.
+    """
+    user_credentials = {'username': 'test', 'password': 'test'}
+
+    @classmethod
+    def setUpTestData(cls):
+        cls.user = User.objects.create_user(
+            email='test@example.com', is_active=False,
+            **cls.user_credentials
+        )
+
+    def test_authenticate(self):
+        self.assertFalse(self.user.is_active)
+        self.assertEqual(authenticate(**self.user_credentials), self.user)
+
+    def test_get_user(self):
+        self.client.force_login(self.user)
+        request = HttpRequest()
+        request.session = self.client.session
+        user = get_user(request)
+        self.assertEqual(user, self.user)

+ 3 - 0
tests/auth_tests/test_forms.py

@@ -166,6 +166,9 @@ class UserCreationFormTest(TestDataMixin, TestCase):
         self.assertEqual(form.cleaned_data['password2'], data['password2'])
 
 
+# To verify that the login form rejects inactive users, use an authentication
+# backend that allows them.
+@override_settings(AUTHENTICATION_BACKENDS=['django.contrib.auth.backends.AllowAllUsersModelBackend'])
 class AuthenticationFormTest(TestDataMixin, TestCase):
 
     def test_invalid_username(self):

+ 15 - 0
tests/auth_tests/test_remote_user.py

@@ -145,6 +145,11 @@ class RemoteUserTest(TestCase):
         # In backends that do not create new users, it is '' (anonymous user)
         self.assertNotEqual(response.context['user'].username, 'knownuser')
 
+    def test_inactive_user(self):
+        User.objects.create(username='knownuser', is_active=False)
+        response = self.client.get('/remote_user/', **{self.header: 'knownuser'})
+        self.assertTrue(response.context['user'].is_anonymous())
+
 
 class RemoteUserNoCreateBackend(RemoteUserBackend):
     """Backend that doesn't create unknown users."""
@@ -166,6 +171,16 @@ class RemoteUserNoCreateTest(RemoteUserTest):
         self.assertEqual(User.objects.count(), num_users)
 
 
+class AllowAllUsersRemoteUserBackendTest(RemoteUserTest):
+    """Backend that allows inactive users."""
+    backend = 'django.contrib.auth.backends.AllowAllUsersRemoteUserBackend'
+
+    def test_inactive_user(self):
+        user = User.objects.create(username='knownuser', is_active=False)
+        response = self.client.get('/remote_user/', **{self.header: self.known_user})
+        self.assertEqual(response.context['user'].username, user.username)
+
+
 class CustomRemoteUserBackend(RemoteUserBackend):
     """
     Backend that overrides RemoteUserBackend methods.

+ 7 - 1
tests/test_client/tests.py

@@ -437,6 +437,12 @@ class ClientTest(TestCase):
         login = self.client.login(username='inactive', password='password')
         self.assertFalse(login)
 
+    @override_settings(
+        AUTHENTICATION_BACKENDS=[
+            'django.contrib.auth.backends.ModelBackend',
+            'django.contrib.auth.backends.AllowAllUsersModelBackend',
+        ]
+    )
     def test_view_with_inactive_force_login(self):
         "Request a page that is protected with @login, but use an inactive login"
 
@@ -445,7 +451,7 @@ class ClientTest(TestCase):
         self.assertRedirects(response, '/accounts/login/?next=/login_protected_view/')
 
         # Log in
-        self.client.force_login(self.u2)
+        self.client.force_login(self.u2, backend='django.contrib.auth.backends.AllowAllUsersModelBackend')
 
         # Request a page that requires a login
         response = self.client.get('/login_protected_view/')