diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index 49c816dc9e..b0635669e9 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -436,7 +436,9 @@ class BaseModelAdmin(metaclass=forms.MediaDefiningClass): else self.get_list_display(request) ) - def lookup_allowed(self, lookup, value): + # RemovedInDjango60Warning: when the deprecation ends, replace with: + # def lookup_allowed(self, lookup, value, request): + def lookup_allowed(self, lookup, value, request=None): from django.contrib.admin.filters import SimpleListFilter model = self.model @@ -482,7 +484,12 @@ class BaseModelAdmin(metaclass=forms.MediaDefiningClass): # Either a local field filter, or no fields at all. return True valid_lookups = {self.date_hierarchy} - for filter_item in self.list_filter: + # RemovedInDjango60Warning: when the deprecation ends, replace with: + # for filter_item in self.get_list_filter(request): + list_filter = ( + self.get_list_filter(request) if request is not None else self.list_filter + ) + for filter_item in list_filter: if isinstance(filter_item, type) and issubclass( filter_item, SimpleListFilter ): diff --git a/django/contrib/admin/views/main.py b/django/contrib/admin/views/main.py index 9a130ae8a7..c99972c2bd 100644 --- a/django/contrib/admin/views/main.py +++ b/django/contrib/admin/views/main.py @@ -1,3 +1,4 @@ +import warnings from datetime import datetime, timedelta from django import forms @@ -31,7 +32,9 @@ from django.core.paginator import InvalidPage from django.db.models import Exists, F, Field, ManyToOneRel, OrderBy, OuterRef from django.db.models.expressions import Combinable from django.urls import reverse +from django.utils.deprecation import RemovedInDjango60Warning from django.utils.http import urlencode +from django.utils.inspect import func_supports_parameter from django.utils.timezone import make_aware from django.utils.translation import gettext @@ -174,9 +177,19 @@ class ChangeList: may_have_duplicates = False has_active_filters = False + supports_request = func_supports_parameter( + self.model_admin.lookup_allowed, "request" + ) + if not supports_request: + warnings.warn( + f"`request` must be added to the signature of " + f"{self.model_admin.__class__.__qualname__}.lookup_allowed().", + RemovedInDjango60Warning, + ) for key, value_list in lookup_params.items(): for value in value_list: - if not self.model_admin.lookup_allowed(key, value): + params = (key, value, request) if supports_request else (key, value) + if not self.model_admin.lookup_allowed(*params): raise DisallowedModelAdminLookup(f"Filtering by {key} not allowed") filter_specs = [] diff --git a/django/contrib/auth/admin.py b/django/contrib/auth/admin.py index 5424711643..f9532abc14 100644 --- a/django/contrib/auth/admin.py +++ b/django/contrib/auth/admin.py @@ -106,10 +106,12 @@ class UserAdmin(admin.ModelAdmin): ), ] + super().get_urls() - def lookup_allowed(self, lookup, value): + # RemovedInDjango60Warning: when the deprecation ends, replace with: + # def lookup_allowed(self, lookup, value, request): + def lookup_allowed(self, lookup, value, request=None): # Don't allow lookups involving passwords. return not lookup.startswith("password") and super().lookup_allowed( - lookup, value + lookup, value, request ) @sensitive_post_parameters_m diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index b151020375..168a5572c6 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -21,6 +21,9 @@ details on these changes. * Support for passing positional arguments to ``BaseConstraint`` will be removed. +* ``request`` will be required in the signature of + ``ModelAdmin.lookup_allowed()`` subclasses. + .. _deprecation-removed-in-5.1: 5.1 diff --git a/docs/ref/contrib/admin/index.txt b/docs/ref/contrib/admin/index.txt index e856f0b640..636d44c750 100644 --- a/docs/ref/contrib/admin/index.txt +++ b/docs/ref/contrib/admin/index.txt @@ -1845,7 +1845,7 @@ templates used by the :class:`ModelAdmin` views: kwargs["formset"] = MyAdminFormSet return super().get_changelist_formset(request, **kwargs) -.. method:: ModelAdmin.lookup_allowed(lookup, value) +.. method:: ModelAdmin.lookup_allowed(lookup, value, request) The objects in the changelist page can be filtered with lookups from the URL's query string. This is how :attr:`list_filter` works, for example. The @@ -1855,10 +1855,11 @@ templates used by the :class:`ModelAdmin` views: unauthorized data exposure. The ``lookup_allowed()`` method is given a lookup path from the query string - (e.g. ``'user__email'``) and the corresponding value - (e.g. ``'user@example.com'``), and returns a boolean indicating whether - filtering the changelist's ``QuerySet`` using the parameters is permitted. - If ``lookup_allowed()`` returns ``False``, ``DisallowedModelAdminLookup`` + (e.g. ``'user__email'``), the corresponding value + (e.g. ``'user@example.com'``), and the request, and returns a boolean + indicating whether filtering the changelist's ``QuerySet`` using the + parameters is permitted. If ``lookup_allowed()`` returns ``False``, + ``DisallowedModelAdminLookup`` (subclass of :exc:`~django.core.exceptions.SuspiciousOperation`) is raised. By default, ``lookup_allowed()`` allows access to a model's local fields, @@ -1870,6 +1871,10 @@ templates used by the :class:`ModelAdmin` views: Override this method to customize the lookups permitted for your :class:`~django.contrib.admin.ModelAdmin` subclass. + .. versionchanged:: 5.0 + + The ``request`` argument was added. + .. method:: ModelAdmin.has_view_permission(request, obj=None) Should return ``True`` if viewing ``obj`` is permitted, ``False`` otherwise. diff --git a/docs/releases/5.0.txt b/docs/releases/5.0.txt index 0d1e7ffda1..934da1bba9 100644 --- a/docs/releases/5.0.txt +++ b/docs/releases/5.0.txt @@ -387,6 +387,10 @@ Miscellaneous :class:`~django.db.models.BaseConstraint` is deprecated in favor of keyword-only arguments. +* ``request`` is added to the signature of :meth:`.ModelAdmin.lookup_allowed`. + Support for ``ModelAdmin`` subclasses that do not accept this argument is + deprecated. + Features removed in 5.0 ======================= diff --git a/tests/modeladmin/tests.py b/tests/modeladmin/tests.py index 5604e39746..627029c9cf 100644 --- a/tests/modeladmin/tests.py +++ b/tests/modeladmin/tests.py @@ -19,8 +19,9 @@ from django.contrib.admin.widgets import ( from django.contrib.auth.models import User from django.db import models from django.forms.widgets import Select -from django.test import SimpleTestCase, TestCase +from django.test import RequestFactory, SimpleTestCase, TestCase from django.test.utils import isolate_apps +from django.utils.deprecation import RemovedInDjango60Warning from .models import Band, Concert, Song @@ -121,7 +122,10 @@ class ModelAdminTests(TestCase): fields = ["name"] ma = BandAdmin(Band, self.site) - self.assertTrue(ma.lookup_allowed("name__nonexistent", "test_value")) + self.assertIs( + ma.lookup_allowed("name__nonexistent", "test_value", request), + True, + ) @isolate_apps("modeladmin") def test_lookup_allowed_onetoone(self): @@ -147,11 +151,15 @@ class ModelAdminTests(TestCase): ma = EmployeeProfileAdmin(EmployeeProfile, self.site) # Reverse OneToOneField self.assertIs( - ma.lookup_allowed("employee__employeeinfo__description", "test_value"), True + ma.lookup_allowed( + "employee__employeeinfo__description", "test_value", request + ), + True, ) # OneToOneField and ForeignKey self.assertIs( - ma.lookup_allowed("employee__department__code", "test_value"), True + ma.lookup_allowed("employee__department__code", "test_value", request), + True, ) @isolate_apps("modeladmin") @@ -175,13 +183,87 @@ class ModelAdminTests(TestCase): ] ma = WaiterAdmin(Waiter, self.site) - self.assertIs(ma.lookup_allowed("restaurant__place__country", "1"), True) self.assertIs( - ma.lookup_allowed("restaurant__place__country__id__exact", "1"), True + ma.lookup_allowed("restaurant__place__country", "1", request), + True, ) self.assertIs( - ma.lookup_allowed("restaurant__place__country__name", "test_value"), True + ma.lookup_allowed("restaurant__place__country__id__exact", "1", request), + True, ) + self.assertIs( + ma.lookup_allowed( + "restaurant__place__country__name", "test_value", request + ), + True, + ) + + def test_lookup_allowed_considers_dynamic_list_filter(self): + class ConcertAdmin(ModelAdmin): + list_filter = ["main_band__sign_date"] + + def get_list_filter(self, request): + if getattr(request, "user", None): + return self.list_filter + ["main_band__name"] + return self.list_filter + + model_admin = ConcertAdmin(Concert, self.site) + request_band_name_filter = RequestFactory().get( + "/", {"main_band__name": "test"} + ) + self.assertIs( + model_admin.lookup_allowed( + "main_band__sign_date", "?", request_band_name_filter + ), + True, + ) + self.assertIs( + model_admin.lookup_allowed( + "main_band__name", "?", request_band_name_filter + ), + False, + ) + request_with_superuser = request + self.assertIs( + model_admin.lookup_allowed( + "main_band__sign_date", "?", request_with_superuser + ), + True, + ) + self.assertIs( + model_admin.lookup_allowed("main_band__name", "?", request_with_superuser), + True, + ) + + def test_lookup_allowed_without_request_deprecation(self): + class ConcertAdmin(ModelAdmin): + list_filter = ["main_band__sign_date"] + + def get_list_filter(self, request): + return self.list_filter + ["main_band__name"] + + def lookup_allowed(self, lookup, value): + return True + + model_admin = ConcertAdmin(Concert, self.site) + msg = ( + "`request` must be added to the signature of ModelAdminTests." + "test_lookup_allowed_without_request_deprecation.." + "ConcertAdmin.lookup_allowed()." + ) + request_band_name_filter = RequestFactory().get( + "/", {"main_band__name": "test"} + ) + request_band_name_filter.user = User.objects.create_superuser( + username="bob", email="bob@test.com", password="test" + ) + with self.assertWarnsMessage(RemovedInDjango60Warning, msg): + changelist = model_admin.get_changelist_instance(request_band_name_filter) + filterspec = changelist.get_filters(request_band_name_filter)[0][0] + self.assertEqual(filterspec.title, "sign date") + filterspec = changelist.get_filters(request_band_name_filter)[0][1] + self.assertEqual(filterspec.title, "name") + self.assertSequenceEqual(filterspec.lookup_choices, [self.band.name]) def test_field_arguments(self): # If fields is specified, fieldsets_add and fieldsets_change should