Browse Source

Fixed #33561 -- Allowed synchronization of user attributes in RemoteUserBackend.

Adrian Torres 3 years ago
parent
commit
d90e34c61b

+ 1 - 0
AUTHORS

@@ -23,6 +23,7 @@ answer newbie questions, and generally made Django that much better:
     Adiyat Mubarak <adiyatmubarak@gmail.com>
     Adnan Umer <u.adnan@outlook.com>
     Adrian Holovaty <adrian@holovaty.com>
+    Adrian Torres <atorresj@redhat.com>
     Adrien Lemaire <lemaire.adrien@gmail.com>
     Afonso Fernández Nogueira <fonzzo.django@gmail.com>
     AgarFu <heaven@croasanaso.sytes.net>

+ 20 - 4
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.
         """

+ 3 - 0
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

+ 15 - 5
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

+ 7 - 0
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
 =======================
 

+ 47 - 4
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.