From db1b10ef0dcab2b8bacbea4adc681a57bd70b462 Mon Sep 17 00:00:00 2001 From: Joshua Cannon Date: Thu, 13 Dec 2018 10:14:03 -0600 Subject: [PATCH] Fixed #30037 -- Added request arg to RemoteUserBackend.configure_user(). --- AUTHORS | 1 + django/contrib/auth/backends.py | 18 ++++++- docs/internals/deprecation.txt | 3 ++ docs/ref/contrib/auth.txt | 11 +++- docs/releases/2.2.txt | 7 ++- tests/auth_tests/test_remote_user.py | 19 +++++-- .../test_remote_user_deprecation.py | 50 +++++++++++++++++++ 7 files changed, 100 insertions(+), 9 deletions(-) create mode 100644 tests/auth_tests/test_remote_user_deprecation.py diff --git a/AUTHORS b/AUTHORS index 51bc2f592a..ddc2c9ee84 100644 --- a/AUTHORS +++ b/AUTHORS @@ -439,6 +439,7 @@ answer newbie questions, and generally made Django that much better: Josef Rousek Joseph Kocherhans Josh Smeaton + Joshua Cannon Joshua Ginsberg Jozko Skrablin J. Pablo Fernandez diff --git a/django/contrib/auth/backends.py b/django/contrib/auth/backends.py index 64937753ed..7549d71273 100644 --- a/django/contrib/auth/backends.py +++ b/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. diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 8c81744e17..0e85e59e8c 100644 --- a/docs/internals/deprecation.txt +++ b/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 diff --git a/docs/ref/contrib/auth.txt b/docs/ref/contrib/auth.txt index 0dfa3a9261..efbffa62c2 100644 --- a/docs/ref/contrib/auth.txt +++ b/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 diff --git a/docs/releases/2.2.txt b/docs/releases/2.2.txt index 150fe413db..7cb5c84475 100644 --- a/docs/releases/2.2.txt +++ b/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. diff --git a/tests/auth_tests/test_remote_user.py b/tests/auth_tests/test_remote_user.py index 2a1ebbb569..6260c460af 100644 --- a/tests/auth_tests/test_remote_user.py +++ b/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') diff --git a/tests/auth_tests/test_remote_user_deprecation.py b/tests/auth_tests/test_remote_user_deprecation.py new file mode 100644 index 0000000000..1b31d9f038 --- /dev/null +++ b/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')