From ceecd518b19044181a3598c55ebed7c2545963cc Mon Sep 17 00:00:00 2001 From: Jaap Roes Date: Thu, 28 Nov 2024 14:42:59 +0100 Subject: [PATCH] Fixed #35530 -- Deprecated request.user fallback in auth.login and auth.alogin. --- django/contrib/auth/__init__.py | 21 +++++++++++++ docs/internals/deprecation.txt | 4 +++ docs/releases/5.2.txt | 4 +++ tests/async/test_async_auth.py | 54 +++++++++++++++++++++++++++++++-- tests/auth_tests/test_login.py | 49 +++++++++++++++++++++++++++++- 5 files changed, 129 insertions(+), 3 deletions(-) diff --git a/django/contrib/auth/__init__.py b/django/contrib/auth/__init__.py index 689567ca6c..8e359ec7ff 100644 --- a/django/contrib/auth/__init__.py +++ b/django/contrib/auth/__init__.py @@ -1,11 +1,13 @@ import inspect import re +import warnings from django.apps import apps as django_apps from django.conf import settings from django.core.exceptions import ImproperlyConfigured, PermissionDenied from django.middleware.csrf import rotate_token from django.utils.crypto import constant_time_compare +from django.utils.deprecation import RemovedInDjango61Warning from django.utils.module_loading import import_string from django.views.decorators.debug import sensitive_variables @@ -154,9 +156,19 @@ def login(request, user, backend=None): have to reauthenticate on every request. Note that data set during the anonymous session is retained when the user logs in. """ + # RemovedInDjango61Warning: When the deprecation ends, replace with: + # session_auth_hash = user.get_session_auth_hash() session_auth_hash = "" + # RemovedInDjango61Warning. if user is None: user = request.user + warnings.warn( + "Fallback to request.user when user is None will be removed.", + RemovedInDjango61Warning, + stacklevel=2, + ) + + # RemovedInDjango61Warning. if hasattr(user, "get_session_auth_hash"): session_auth_hash = user.get_session_auth_hash() @@ -187,9 +199,18 @@ def login(request, user, backend=None): async def alogin(request, user, backend=None): """See login().""" + # RemovedInDjango61Warning: When the deprecation ends, replace with: + # session_auth_hash = user.get_session_auth_hash() session_auth_hash = "" + # RemovedInDjango61Warning. if user is None: + warnings.warn( + "Fallback to request.user when user is None will be removed.", + RemovedInDjango61Warning, + stacklevel=2, + ) user = await request.auser() + # RemovedInDjango61Warning. if hasattr(user, "get_session_auth_hash"): session_auth_hash = user.get_session_auth_hash() diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 85ad0d400f..171d9ecbe3 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -18,6 +18,10 @@ details on these changes. * The ``all`` keyword argument of ``django.contrib.staticfiles.finders.find()`` will be removed. +* The fallback to ``request.user`` when ``user`` is ``None`` in + ``django.contrib.auth.login()`` and ``django.contrib.auth.alogin()`` will be + removed. + .. _deprecation-removed-in-6.0: 6.0 diff --git a/docs/releases/5.2.txt b/docs/releases/5.2.txt index 2ec90429d0..7af0b955f6 100644 --- a/docs/releases/5.2.txt +++ b/docs/releases/5.2.txt @@ -443,3 +443,7 @@ Miscellaneous * The ``all`` argument for the ``django.contrib.staticfiles.finders.find()`` function is deprecated in favor of the ``find_all`` argument. + +* The fallback to ``request.user`` when ``user`` is ``None`` in + ``django.contrib.auth.login()`` and ``django.contrib.auth.alogin()`` will be + removed. diff --git a/tests/async/test_async_auth.py b/tests/async/test_async_auth.py index 37884d13a6..3d5a6b678d 100644 --- a/tests/async/test_async_auth.py +++ b/tests/async/test_async_auth.py @@ -8,6 +8,7 @@ from django.contrib.auth import ( from django.contrib.auth.models import AnonymousUser, User from django.http import HttpRequest from django.test import TestCase, override_settings +from django.utils.deprecation import RemovedInDjango61Warning class AsyncAuthTest(TestCase): @@ -60,7 +61,52 @@ class AsyncAuthTest(TestCase): self.assertIsInstance(user, User) self.assertEqual(user.username, second_user.username) - async def test_alogin_without_user(self): + # RemovedInDjango61Warning: When the deprecation ends, replace with: + # async def test_alogin_without_user(self): + async def test_alogin_without_user_no_request_user(self): + request = HttpRequest() + request.session = await self.client.asession() + # RemovedInDjango61Warning: When the deprecation ends, replace with: + # with self.assertRaisesMessage( + # AttributeError, + # "'NoneType' object has no attribute 'get_session_auth_hash'", + # ): + # await alogin(request, None) + with ( + self.assertRaisesMessage( + AttributeError, + "'HttpRequest' object has no attribute 'auser'", + ), + self.assertWarnsMessage( + RemovedInDjango61Warning, + "Fallback to request.user when user is None will be removed.", + ), + ): + await alogin(request, None) + + # RemovedInDjango61Warning: When the deprecation ends, remove completely. + async def test_alogin_without_user_anonymous_request(self): + async def auser(): + return AnonymousUser() + + request = HttpRequest() + request.user = AnonymousUser() + request.auser = auser + request.session = await self.client.asession() + with ( + self.assertRaisesMessage( + AttributeError, + "'AnonymousUser' object has no attribute '_meta'", + ), + self.assertWarnsMessage( + RemovedInDjango61Warning, + "Fallback to request.user when user is None will be removed.", + ), + ): + await alogin(request, None) + + # RemovedInDjango61Warning: When the deprecation ends, remove completely. + async def test_alogin_without_user_authenticated_request(self): async def auser(): return self.test_user @@ -68,7 +114,11 @@ class AsyncAuthTest(TestCase): request.user = self.test_user request.auser = auser request.session = await self.client.asession() - await alogin(request, None) + with self.assertWarnsMessage( + RemovedInDjango61Warning, + "Fallback to request.user when user is None will be removed.", + ): + await alogin(request, None) user = await aget_user(request) self.assertIsInstance(user, User) self.assertEqual(user.username, self.test_user.username) diff --git a/tests/auth_tests/test_login.py b/tests/auth_tests/test_login.py index 607833a095..2c0c1c5796 100644 --- a/tests/auth_tests/test_login.py +++ b/tests/auth_tests/test_login.py @@ -1,7 +1,8 @@ from django.contrib import auth -from django.contrib.auth.models import User +from django.contrib.auth.models import AnonymousUser, User from django.http import HttpRequest from django.test import TestCase +from django.utils.deprecation import RemovedInDjango61Warning class TestLogin(TestCase): @@ -23,3 +24,49 @@ class TestLogin(TestCase): auth.login(self.request, self.user) self.assertEqual(self.request.session[auth.SESSION_KEY], str(self.user.pk)) + + # RemovedInDjango61Warning: When the deprecation ends, replace with: + # def test_without_user(self): + def test_without_user_no_request_user(self): + # RemovedInDjango61Warning: When the deprecation ends, replace with: + # with self.assertRaisesMessage( + # AttributeError, + # "'NoneType' object has no attribute 'get_session_auth_hash'", + # ): + # auth.login(self.request, None) + with ( + self.assertRaisesMessage( + AttributeError, + "'HttpRequest' object has no attribute 'user'", + ), + self.assertWarnsMessage( + RemovedInDjango61Warning, + "Fallback to request.user when user is None will be removed.", + ), + ): + auth.login(self.request, None) + + # RemovedInDjango61Warning: When the deprecation ends, remove completely. + def test_without_user_anonymous_request(self): + self.request.user = AnonymousUser() + with ( + self.assertRaisesMessage( + AttributeError, + "'AnonymousUser' object has no attribute '_meta'", + ), + self.assertWarnsMessage( + RemovedInDjango61Warning, + "Fallback to request.user when user is None will be removed.", + ), + ): + auth.login(self.request, None) + + # RemovedInDjango61Warning: When the deprecation ends, remove completely. + def test_without_user_authenticated_request(self): + self.request.user = self.user + self.assertNotIn(auth.SESSION_KEY, self.request.session) + + msg = "Fallback to request.user when user is None will be removed." + with self.assertWarnsMessage(RemovedInDjango61Warning, msg): + auth.login(self.request, None) + self.assertEqual(self.request.session[auth.SESSION_KEY], str(self.user.pk))