From 4b9330ccc04575f9e5126529ec355a450d12e77c Mon Sep 17 00:00:00 2001 From: Aleksej Manaev Date: Mon, 11 Jul 2016 16:40:39 +0200 Subject: [PATCH] Fixed #25187 -- Made request available in authentication backends. --- django/contrib/auth/__init__.py | 24 +++++++++++---- django/contrib/auth/backends.py | 4 +-- django/contrib/auth/forms.py | 2 +- django/contrib/auth/middleware.py | 2 +- docs/internals/deprecation.txt | 3 ++ docs/ref/contrib/auth.txt | 16 ++++++++-- docs/releases/1.11.txt | 8 +++++ docs/topics/auth/customizing.txt | 24 ++++++++++----- docs/topics/auth/default.txt | 11 +++++-- tests/auth_tests/test_auth_backends.py | 4 +-- .../test_auth_backends_deprecation.py | 30 +++++++++++++++++++ tests/test_client_regress/auth_backends.py | 2 +- 12 files changed, 106 insertions(+), 24 deletions(-) create mode 100644 tests/auth_tests/test_auth_backends_deprecation.py diff --git a/django/contrib/auth/__init__.py b/django/contrib/auth/__init__.py index fa0ef39a329..77458e15f9a 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 RemovedInDjango21Warning from django.utils.module_loading import import_string from django.utils.translation import LANGUAGE_SESSION_KEY @@ -59,19 +61,29 @@ def _get_user_session_key(request): return get_user_model()._meta.pk.to_python(request.session[SESSION_KEY]) -def authenticate(**credentials): +def authenticate(request=None, **credentials): """ If the given credentials are valid, return a User object. """ for backend, backend_path in _get_backends(return_tuples=True): + args = (request,) try: - inspect.getcallargs(backend.authenticate, **credentials) + inspect.getcallargs(backend.authenticate, request, **credentials) except TypeError: - # This backend doesn't accept these credentials as arguments. Try the next one. - continue - + try: + inspect.getcallargs(backend.authenticate, **credentials) + except TypeError: + # This backend doesn't accept these credentials as arguments. Try the next one. + continue + else: + args = () + warnings.warn( + "Update authentication backend %s to accept a " + "positional `request` argument." % backend_path, + RemovedInDjango21Warning + ) try: - user = backend.authenticate(**credentials) + user = backend.authenticate(*args, **credentials) except PermissionDenied: # This backend says to stop in our tracks - this user should not be allowed in at all. break diff --git a/django/contrib/auth/backends.py b/django/contrib/auth/backends.py index f145a1929a7..379f1c44503 100644 --- a/django/contrib/auth/backends.py +++ b/django/contrib/auth/backends.py @@ -9,7 +9,7 @@ class ModelBackend(object): Authenticates against settings.AUTH_USER_MODEL. """ - def authenticate(self, username=None, password=None, **kwargs): + def authenticate(self, request, username=None, password=None, **kwargs): UserModel = get_user_model() if username is None: username = kwargs.get(UserModel.USERNAME_FIELD) @@ -125,7 +125,7 @@ class RemoteUserBackend(ModelBackend): # Create a User object if not already in the database? create_unknown_user = True - def authenticate(self, remote_user): + def authenticate(self, request, remote_user): """ The username passed as ``remote_user`` is considered trusted. This method simply returns the ``User`` object with the given username, diff --git a/django/contrib/auth/forms.py b/django/contrib/auth/forms.py index ddd915ae691..7a6a8d74aa7 100644 --- a/django/contrib/auth/forms.py +++ b/django/contrib/auth/forms.py @@ -189,7 +189,7 @@ class AuthenticationForm(forms.Form): password = self.cleaned_data.get('password') if username is not None and password: - self.user_cache = authenticate(username=username, password=password) + self.user_cache = authenticate(self.request, username=username, password=password) if self.user_cache is None: raise forms.ValidationError( self.error_messages['invalid_login'], diff --git a/django/contrib/auth/middleware.py b/django/contrib/auth/middleware.py index 8c56e645fe6..f179894e731 100644 --- a/django/contrib/auth/middleware.py +++ b/django/contrib/auth/middleware.py @@ -88,7 +88,7 @@ class RemoteUserMiddleware(MiddlewareMixin): # We are seeing this user for the first time in this session, attempt # to authenticate the user. - user = auth.authenticate(remote_user=username) + user = auth.authenticate(request, remote_user=username) if user: # User is valid. Set request.user and persist user in the session # by logging the user in. diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 8be137e6350..a1c55646d0b 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -38,6 +38,9 @@ details on these changes. * ``DatabaseIntrospection.get_indexes()`` will be removed. +* The ``authenticate()`` method of authentication backends will require a + ``request`` argument. + .. _deprecation-removed-in-2.0: 2.0 diff --git a/docs/ref/contrib/auth.txt b/docs/ref/contrib/auth.txt index f12b16daa0d..38f18b6dc80 100644 --- a/docs/ref/contrib/auth.txt +++ b/docs/ref/contrib/auth.txt @@ -518,7 +518,7 @@ The following backends are available in :mod:`django.contrib.auth.backends`: implement them other than returning an empty set of permissions if ``obj is not None``. - .. method:: authenticate(username=None, password=None, **kwargs) + .. method:: authenticate(request, username=None, password=None, **kwargs) Tries to authenticate ``username`` with ``password`` by calling :meth:`User.check_password @@ -528,6 +528,14 @@ The following backends are available in :mod:`django.contrib.auth.backends`: `. Returns an authenticated user or ``None``. + ``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:: 1.11 + + The ``request`` argument was added. + .. method:: get_user_permissions(user_obj, obj=None) Returns the set of permission strings the ``user_obj`` has from their @@ -603,7 +611,7 @@ The following backends are available in :mod:`django.contrib.auth.backends`: :class:`~django.contrib.auth.models.User` object is created if not already in the database. Defaults to ``True``. -.. method:: RemoteUserBackend.authenticate(remote_user) +.. method:: RemoteUserBackend.authenticate(request, remote_user) The username passed as ``remote_user`` is considered trusted. This method simply returns the ``User`` object with the given username, creating a new @@ -614,6 +622,10 @@ The following backends are available in :mod:`django.contrib.auth.backends`: ``False`` and a ``User`` object with the given username is not found in the database. + ``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). + .. method:: RemoteUserBackend.clean_username(username) Performs any cleaning on the ``username`` (e.g. stripping LDAP DN diff --git a/docs/releases/1.11.txt b/docs/releases/1.11.txt index 7b5bcc450f5..a6edfcdf25a 100644 --- a/docs/releases/1.11.txt +++ b/docs/releases/1.11.txt @@ -113,6 +113,10 @@ Minor features allows using the authentication system :ref:`without any of the built-in models `. +* The ``HttpRequest`` is now passed to :func:`~django.contrib.auth.authenticate` + which in turn passes it to the authentication backend if it accepts a + ``request`` argument. + :mod:`django.contrib.contenttypes` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -556,3 +560,7 @@ Miscellaneous * ``DatabaseIntrospection.get_indexes()`` is deprecated in favor of ``DatabaseIntrospection.get_constraints()``. + +* :func:`~django.contrib.auth.authenticate` now passes a ``request`` argument + to the ``authenticate()`` method of authentication backends. Support for + methods that don't accept ``request`` will be removed in Django 2.1. diff --git a/docs/topics/auth/customizing.txt b/docs/topics/auth/customizing.txt index d102d9939b4..085f74fcde0 100644 --- a/docs/topics/auth/customizing.txt +++ b/docs/topics/auth/customizing.txt @@ -89,25 +89,26 @@ Writing an authentication backend --------------------------------- An authentication backend is a class that implements two required methods: -``get_user(user_id)`` and ``authenticate(**credentials)``, as well as a set of -optional permission related :ref:`authorization methods `. +``get_user(user_id)`` and ``authenticate(request, **credentials)``, as well as +a set of optional permission related :ref:`authorization methods +`. The ``get_user`` method takes a ``user_id`` -- which could be a username, database ID or whatever, but has to be the primary key of your ``User`` object -- and returns a ``User`` object. -The ``authenticate`` method takes credentials as keyword arguments. Most of -the time, it'll just look like this:: +The ``authenticate`` method takes a ``request`` argument and credentials as +keyword arguments. Most of the time, it'll just look like this:: class MyBackend(object): - def authenticate(self, username=None, password=None): + def authenticate(self, request, username=None, password=None): # Check the username/password and return a User. ... But it could also authenticate a token, like so:: class MyBackend(object): - def authenticate(self, token=None): + def authenticate(self, request, token=None): # Check the token and return a User. ... @@ -115,6 +116,10 @@ Either way, ``authenticate`` should check the credentials it gets, and it should return a ``User`` object that matches those credentials, if the credentials are valid. If they're not valid, it should return ``None``. +``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). + The Django admin is tightly coupled to the Django :ref:`User object `. The best way to deal with this is to create a Django ``User`` object for each user that exists for your backend (e.g., in your LDAP @@ -140,7 +145,7 @@ object the first time a user authenticates:: ADMIN_PASSWORD = 'pbkdf2_sha256$30000$Vo0VlMnkR4Bk$qEvtdyZRWTcOsCnI/oQ7fVOu1XAURIZYoOZ3iq8Dr4M=' """ - def authenticate(self, username=None, password=None): + def authenticate(self, request, username=None, password=None): login_valid = (settings.ADMIN_LOGIN == username) pwd_valid = check_password(password, settings.ADMIN_PASSWORD) if login_valid and pwd_valid: @@ -163,6 +168,11 @@ object the first time a user authenticates:: except User.DoesNotExist: return None +.. versionchanged:: 1.11 + + The ``request`` parameter was added to ``authenticate()`` and support for + backends that don't accept it will be removed in Django 2.1. + .. _authorization_methods: Handling authorization in custom backends diff --git a/docs/topics/auth/default.txt b/docs/topics/auth/default.txt index a484a89d6df..310f09e322b 100644 --- a/docs/topics/auth/default.txt +++ b/docs/topics/auth/default.txt @@ -115,7 +115,7 @@ Changing a user's password will log out all their sessions. See Authenticating users -------------------- -.. function:: authenticate(\**credentials) +.. function:: authenticate(request=None, \**credentials) Use :func:`~django.contrib.auth.authenticate()` to verify a set of credentials. It takes credentials as keyword arguments, ``username`` and @@ -133,6 +133,13 @@ Authenticating users else: # No backend authenticated the credentials + ``request`` is an optional :class:`~django.http.HttpRequest` which is + passed on the ``authenticate()`` method of the authentication backends. + + .. versionchanged:: 1.11 + + The optional ``request`` argument was added. + .. note:: This is a low level way to authenticate a set of credentials; for @@ -342,7 +349,7 @@ If you have an authenticated user you want to attach to the current session def my_view(request): username = request.POST['username'] password = request.POST['password'] - user = authenticate(username=username, password=password) + user = authenticate(request, username=username, password=password) if user is not None: login(request, user) # Redirect to a success page. diff --git a/tests/auth_tests/test_auth_backends.py b/tests/auth_tests/test_auth_backends.py index e850772f35b..ea40992a617 100644 --- a/tests/auth_tests/test_auth_backends.py +++ b/tests/auth_tests/test_auth_backends.py @@ -475,7 +475,7 @@ class PermissionDeniedBackend(object): Always raises PermissionDenied in `authenticate`, `has_perm` and `has_module_perms`. """ - def authenticate(self, username=None, password=None): + def authenticate(self, request, username=None, password=None): raise PermissionDenied def has_perm(self, user_obj, perm, obj=None): @@ -585,7 +585,7 @@ class TypeErrorBackend(object): Always raises TypeError. """ - def authenticate(self, username=None, password=None): + def authenticate(self, request, username=None, password=None): raise TypeError diff --git a/tests/auth_tests/test_auth_backends_deprecation.py b/tests/auth_tests/test_auth_backends_deprecation.py new file mode 100644 index 00000000000..61789365356 --- /dev/null +++ b/tests/auth_tests/test_auth_backends_deprecation.py @@ -0,0 +1,30 @@ +import warnings + +from django.contrib.auth import authenticate +from django.test import SimpleTestCase, override_settings + + +class NoRequestBackend(object): + def authenticate(self, username=None, password=None): + # Doesn't accept a request parameter. + pass + + +class AcceptsRequestBackendTest(SimpleTestCase): + """ + A deprecation warning is shown for backends that have an authenticate() + method without a request parameter. + """ + no_request_backend = '%s.NoRequestBackend' % __name__ + + @override_settings(AUTHENTICATION_BACKENDS=[no_request_backend]) + def test_no_request_deprecation_warning(self): + with warnings.catch_warnings(record=True) as warns: + warnings.simplefilter('always') + authenticate(username='test', password='test') + self.assertEqual(len(warns), 1) + self.assertEqual( + str(warns[0].message), + "Update authentication backend %s to accept a positional `request` " + "argument." % self.no_request_backend + ) diff --git a/tests/test_client_regress/auth_backends.py b/tests/test_client_regress/auth_backends.py index 02a6d396119..847bef85f08 100644 --- a/tests/test_client_regress/auth_backends.py +++ b/tests/test_client_regress/auth_backends.py @@ -5,7 +5,7 @@ from .models import CustomUser class CustomUserBackend(ModelBackend): - def authenticate(self, username=None, password=None): + def authenticate(self, request, username=None, password=None): try: user = CustomUser.custom_objects.get_by_natural_key(username) if user.check_password(password):