Browse Source

Fixed #30037 -- Added request arg to RemoteUserBackend.configure_user().

Joshua Cannon 6 years ago
parent
commit
db1b10ef0d

+ 1 - 0
AUTHORS

@@ -439,6 +439,7 @@ answer newbie questions, and generally made Django that much better:
     Josef Rousek <josef.rousek@gmail.com>
     Joseph Kocherhans <joseph@jkocherhans.com>
     Josh Smeaton <josh.smeaton@gmail.com>
+    Joshua Cannon <joshdcannon@gmail.com>
     Joshua Ginsberg <jag@flowtheory.net>
     Jozko Skrablin <jozko.skrablin@gmail.com>
     J. Pablo Fernandez <pupeno@pupeno.com>

+ 16 - 2
django/contrib/auth/backends.py

@@ -1,5 +1,9 @@
+import inspect
+import warnings
+
 from django.contrib.auth import get_user_model
 from django.contrib.auth.models import Permission
+from django.utils.deprecation import RemovedInDjango31Warning
 
 UserModel = get_user_model()
 
@@ -143,7 +147,17 @@ class RemoteUserBackend(ModelBackend):
                 UserModel.USERNAME_FIELD: username
             })
             if created:
-                user = self.configure_user(user)
+                args = (request, user)
+                try:
+                    inspect.getcallargs(self.configure_user, request, user)
+                except TypeError:
+                    args = (user,)
+                    warnings.warn(
+                        'Update %s.configure_user() to accept `request` as '
+                        'the first argument.'
+                        % self.__class__.__name__, RemovedInDjango31Warning
+                    )
+                user = self.configure_user(*args)
         else:
             try:
                 user = UserModel._default_manager.get_by_natural_key(username)
@@ -160,7 +174,7 @@ class RemoteUserBackend(ModelBackend):
         """
         return username
 
-    def configure_user(self, user):
+    def configure_user(self, request, user):
         """
         Configure a user after creation and return the updated user.
 

+ 3 - 0
docs/internals/deprecation.txt

@@ -29,6 +29,9 @@ details on these changes.
 * ``django.contrib.staticfiles.storage.CachedStaticFilesStorage`` will be
   removed.
 
+* ``RemoteUserBackend.configure_user()`` will require ``request`` as the first
+  positional argument.
+
 .. _deprecation-removed-in-3.0:
 
 3.0

+ 10 - 1
docs/ref/contrib/auth.txt

@@ -611,13 +611,22 @@ 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(user)
+    .. method:: configure_user(request, user)
 
         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.
 
+        ``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:: 2.2
+
+            The ``request`` argument was added. Support for method overrides
+            that don't accept it will be removed in Django 3.1.
+
     .. method:: user_can_authenticate()
 
         Returns whether the user is allowed to authenticate. This method

+ 6 - 1
docs/releases/2.2.txt

@@ -55,7 +55,8 @@ Minor features
 :mod:`django.contrib.auth`
 ~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-* ...
+* The ``HttpRequest`` is now passed as the first positional argument to
+  :meth:`.RemoteUserBackend.configure_user`, if it accepts it.
 
 :mod:`django.contrib.contenttypes`
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -505,3 +506,7 @@ Miscellaneous
 * ``django.contrib.staticfiles.storage.CachedStaticFilesStorage`` is
   deprecated due to the intractable problems that is has. Use
   :class:`.ManifestStaticFilesStorage` or a third-party cloud storage instead.
+
+* :meth:`.RemoteUserBackend.configure_user` is now passed ``request`` as the
+  first positional argument, if it accepts it. Support for overrides that don't
+  accept it will be removed in Django 3.1.

+ 14 - 5
tests/auth_tests/test_remote_user.py

@@ -15,6 +15,7 @@ class RemoteUserTest(TestCase):
     middleware = 'django.contrib.auth.middleware.RemoteUserMiddleware'
     backend = 'django.contrib.auth.backends.RemoteUserBackend'
     header = 'REMOTE_USER'
+    email_header = 'REMOTE_EMAIL'
 
     # Usernames to be passed in REMOTE_USER for the test_known_user test case.
     known_user = 'knownuser'
@@ -192,11 +193,11 @@ class CustomRemoteUserBackend(RemoteUserBackend):
         """
         return username.split('@')[0]
 
-    def configure_user(self, user):
+    def configure_user(self, request, user):
         """
-        Sets user's email address.
+        Sets user's email address using the email specified in an HTTP header.
         """
-        user.email = 'user@example.com'
+        user.email = request.META.get(RemoteUserTest.email_header, '')
         user.save()
         return user
 
@@ -224,9 +225,17 @@ class RemoteUserCustomTest(RemoteUserTest):
 
     def test_unknown_user(self):
         """
-        The unknown user created should be configured with an email address.
+        The unknown user created should be configured with an email address
+        provided in the request header.
         """
-        super().test_unknown_user()
+        num_users = User.objects.count()
+        response = self.client.get('/remote_user/', **{
+            self.header: 'newuser',
+            self.email_header: 'user@example.com',
+        })
+        self.assertEqual(response.context['user'].username, 'newuser')
+        self.assertEqual(response.context['user'].email, 'user@example.com')
+        self.assertEqual(User.objects.count(), num_users + 1)
         newuser = User.objects.get(username='newuser')
         self.assertEqual(newuser.email, 'user@example.com')
 

+ 50 - 0
tests/auth_tests/test_remote_user_deprecation.py

@@ -0,0 +1,50 @@
+import warnings
+
+from django.contrib.auth.backends import RemoteUserBackend
+from django.contrib.auth.models import User
+from django.test import TestCase, modify_settings, override_settings
+
+
+class CustomRemoteUserBackend(RemoteUserBackend):
+    """Override configure_user() without a request argument."""
+    def configure_user(self, user):
+        user.email = 'user@example.com'
+        user.save()
+        return user
+
+
+@override_settings(ROOT_URLCONF='auth_tests.urls')
+class RemoteUserCustomTest(TestCase):
+    middleware = 'django.contrib.auth.middleware.RemoteUserMiddleware'
+    backend = 'auth_tests.test_remote_user_deprecation.CustomRemoteUserBackend'
+    header = 'REMOTE_USER'
+
+    def setUp(self):
+        self.patched_settings = modify_settings(
+            AUTHENTICATION_BACKENDS={'append': self.backend},
+            MIDDLEWARE={'append': self.middleware},
+        )
+        self.patched_settings.enable()
+
+    def tearDown(self):
+        self.patched_settings.disable()
+
+    def test_configure_user_deprecation_warning(self):
+        """
+        A deprecation warning is shown for RemoteUserBackend that have a
+        configure_user() method without a request parameter.
+        """
+        num_users = User.objects.count()
+        with warnings.catch_warnings(record=True) as warns:
+            warnings.simplefilter('always')
+            response = self.client.get('/remote_user/', **{self.header: 'newuser'})
+            self.assertEqual(response.context['user'].username, 'newuser')
+        self.assertEqual(len(warns), 1)
+        self.assertEqual(
+            str(warns[0].message),
+            'Update CustomRemoteUserBackend.configure_user() to accept '
+            '`request` as the first argument.'
+        )
+        self.assertEqual(User.objects.count(), num_users + 1)
+        user = User.objects.get(username='newuser')
+        self.assertEqual(user.email, 'user@example.com')