From d90e34c61b27fba2527834806639eebbcfab9631 Mon Sep 17 00:00:00 2001 From: Adrian Torres Date: Fri, 4 Mar 2022 11:04:07 +0100 Subject: [PATCH] Fixed #33561 -- Allowed synchronization of user attributes in RemoteUserBackend. --- AUTHORS | 1 + django/contrib/auth/backends.py | 24 ++++++++++--- docs/internals/deprecation.txt | 3 ++ docs/ref/contrib/auth.txt | 20 ++++++++--- docs/releases/4.1.txt | 7 ++++ tests/auth_tests/test_remote_user.py | 51 +++++++++++++++++++++++++--- 6 files changed, 93 insertions(+), 13 deletions(-) diff --git a/AUTHORS b/AUTHORS index 9f2475def7..1f96c6700c 100644 --- a/AUTHORS +++ b/AUTHORS @@ -23,6 +23,7 @@ answer newbie questions, and generally made Django that much better: Adiyat Mubarak Adnan Umer Adrian Holovaty + Adrian Torres Adrien Lemaire Afonso Fernández Nogueira AgarFu diff --git a/django/contrib/auth/backends.py b/django/contrib/auth/backends.py index 7cf405713d..1e12efac38 100644 --- a/django/contrib/auth/backends.py +++ b/django/contrib/auth/backends.py @@ -1,6 +1,10 @@ +import warnings + from django.contrib.auth import get_user_model from django.contrib.auth.models import Permission from django.db.models import Exists, OuterRef, Q +from django.utils.deprecation import RemovedInDjango50Warning +from django.utils.inspect import func_supports_parameter UserModel = get_user_model() @@ -192,6 +196,7 @@ class RemoteUserBackend(ModelBackend): """ if not remote_user: return + created = False user = None username = self.clean_username(remote_user) @@ -202,13 +207,24 @@ class RemoteUserBackend(ModelBackend): user, created = UserModel._default_manager.get_or_create( **{UserModel.USERNAME_FIELD: username} ) - if created: - user = self.configure_user(request, user) else: try: user = UserModel._default_manager.get_by_natural_key(username) except UserModel.DoesNotExist: pass + + # RemovedInDjango50Warning: When the deprecation ends, replace with: + # user = self.configure_user(request, user, created=created) + if func_supports_parameter(self.configure_user, "created"): + user = self.configure_user(request, user, created=created) + else: + warnings.warn( + f"`created=True` must be added to the signature of " + f"{self.__class__.__qualname__}.configure_user().", + category=RemovedInDjango50Warning, + ) + if created: + user = self.configure_user(request, user) return user if self.user_can_authenticate(user) else None def clean_username(self, username): @@ -220,9 +236,9 @@ class RemoteUserBackend(ModelBackend): """ return username - def configure_user(self, request, user): + def configure_user(self, request, user, created=True): """ - Configure a user after creation and return the updated user. + Configure a user and return the updated user. By default, return the user unmodified. """ diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 11f69b1844..2c85eafc85 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -87,6 +87,9 @@ details on these changes. * Passing unsaved model instances to related filters will no longer be allowed. +* ``created=True`` will be required in the signature of + ``RemoteUserBackend.configure_user()`` subclasses. + .. _deprecation-removed-in-4.1: 4.1 diff --git a/docs/ref/contrib/auth.txt b/docs/ref/contrib/auth.txt index 40c3c84e75..abcf3905c9 100644 --- a/docs/ref/contrib/auth.txt +++ b/docs/ref/contrib/auth.txt @@ -649,17 +649,27 @@ The following backends are available in :mod:`django.contrib.auth.backends`: information) prior to using it to get or create a user object. Returns the cleaned username. - .. method:: configure_user(request, user) + .. method:: configure_user(request, user, created=True) - Configures a newly created user. This method is called immediately - after a 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. + Configures the user on each authentication attempt. This method is + called immediately after fetching or creating the user being + authenticated, 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. + + The setup can be performed either once when the user is created + (``created`` is ``True``) or on existing users (``created`` is + ``False``) as a way of synchronizing attributes between the remote and + the local systems. ``request`` is an :class:`~django.http.HttpRequest` and may be ``None`` if it wasn't provided to :func:`~django.contrib.auth.authenticate` (which passes it on to the backend). + .. versionchanged:: 4.1 + + The ``created`` argument was added. + .. method:: user_can_authenticate() Returns whether the user is allowed to authenticate. This method diff --git a/docs/releases/4.1.txt b/docs/releases/4.1.txt index 72c1c15c7a..9dff4784cb 100644 --- a/docs/releases/4.1.txt +++ b/docs/releases/4.1.txt @@ -74,6 +74,9 @@ Minor features * The default iteration count for the PBKDF2 password hasher is increased from 320,000 to 390,000. +* The :meth:`.RemoteUserBackend.configure_user` method now allows synchronizing + user attributes with attributes in a remote system such as an LDAP directory. + :mod:`django.contrib.contenttypes` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -493,6 +496,10 @@ Miscellaneous * Passing unsaved model instances to related filters is deprecated. In Django 5.0, the exception will be raised. +* ``created=True`` is added to the signature of + :meth:`.RemoteUserBackend.configure_user`. Support for ``RemoteUserBackend`` + subclasses that do not accept this argument is deprecated. + Features removed in 4.1 ======================= diff --git a/tests/auth_tests/test_remote_user.py b/tests/auth_tests/test_remote_user.py index bd974cb1d5..8fb1b972b5 100644 --- a/tests/auth_tests/test_remote_user.py +++ b/tests/auth_tests/test_remote_user.py @@ -6,8 +6,15 @@ from django.contrib.auth.backends import RemoteUserBackend from django.contrib.auth.middleware import RemoteUserMiddleware from django.contrib.auth.models import User from django.middleware.csrf import _get_new_csrf_string, _mask_cipher_secret -from django.test import Client, TestCase, modify_settings, override_settings +from django.test import ( + Client, + TestCase, + ignore_warnings, + modify_settings, + override_settings, +) from django.utils import timezone +from django.utils.deprecation import RemovedInDjango50Warning @override_settings(ROOT_URLCONF="auth_tests.urls") @@ -215,11 +222,14 @@ class CustomRemoteUserBackend(RemoteUserBackend): """ return username.split("@")[0] - def configure_user(self, request, user): + def configure_user(self, request, user, created=True): """ Sets user's email address using the email specified in an HTTP header. + Sets user's last name for existing users. """ user.email = request.META.get(RemoteUserTest.email_header, "") + if not created: + user.last_name = user.username user.save() return user @@ -242,8 +252,12 @@ class RemoteUserCustomTest(RemoteUserTest): should not have been configured with an email address. """ super().test_known_user() - self.assertEqual(User.objects.get(username="knownuser").email, "") - self.assertEqual(User.objects.get(username="knownuser2").email, "") + knownuser = User.objects.get(username="knownuser") + knownuser2 = User.objects.get(username="knownuser2") + self.assertEqual(knownuser.email, "") + self.assertEqual(knownuser2.email, "") + self.assertEqual(knownuser.last_name, "knownuser") + self.assertEqual(knownuser2.last_name, "knownuser2") def test_unknown_user(self): """ @@ -260,11 +274,40 @@ class RemoteUserCustomTest(RemoteUserTest): ) self.assertEqual(response.context["user"].username, "newuser") self.assertEqual(response.context["user"].email, "user@example.com") + self.assertEqual(response.context["user"].last_name, "") self.assertEqual(User.objects.count(), num_users + 1) newuser = User.objects.get(username="newuser") self.assertEqual(newuser.email, "user@example.com") +# RemovedInDjango50Warning. +class CustomRemoteUserNoCreatedArgumentBackend(CustomRemoteUserBackend): + def configure_user(self, request, user): + return super().configure_user(request, user) + + +@ignore_warnings(category=RemovedInDjango50Warning) +class RemoteUserCustomNoCreatedArgumentTest(RemoteUserTest): + backend = "auth_tests.test_remote_user.CustomRemoteUserNoCreatedArgumentBackend" + + +@override_settings(ROOT_URLCONF="auth_tests.urls") +@modify_settings( + AUTHENTICATION_BACKENDS={ + "append": "auth_tests.test_remote_user.CustomRemoteUserNoCreatedArgumentBackend" + }, + MIDDLEWARE={"append": "django.contrib.auth.middleware.RemoteUserMiddleware"}, +) +class RemoteUserCustomNoCreatedArgumentDeprecationTest(TestCase): + def test_known_user_sync(self): + msg = ( + "`created=True` must be added to the signature of " + "CustomRemoteUserNoCreatedArgumentBackend.configure_user()." + ) + with self.assertWarnsMessage(RemovedInDjango50Warning, msg): + self.client.get("/remote_user/", **{RemoteUserTest.header: "newuser"}) + + class CustomHeaderMiddleware(RemoteUserMiddleware): """ Middleware that overrides custom HTTP auth user header.