Fixed #31747 -- Fixed model enumeration via admin URLs.

Co-authored-by: Carlton Gibson <carlton.gibson@noumenal.es>

django/contrib/admin/sites.py

@@ -3,19 +3,23 @@ from functools import update_wrapper
 from weakref import WeakSet
 
 from django.apps import apps
+from django.conf import settings
 from django.contrib.admin import ModelAdmin, actions
 from django.contrib.admin.views.autocomplete import AutocompleteJsonView
 from django.contrib.auth import REDIRECT_FIELD_NAME
 from django.core.exceptions import ImproperlyConfigured
 from django.db.models.base import ModelBase
-from django.http import Http404, HttpResponseRedirect
+from django.http import (
+    Http404, HttpResponsePermanentRedirect, HttpResponseRedirect,
+)
 from django.template.response import TemplateResponse
-from django.urls import NoReverseMatch, reverse
+from django.urls import NoReverseMatch, Resolver404, resolve, reverse
 from django.utils.functional import LazyObject
 from django.utils.module_loading import import_string
 from django.utils.text import capfirst
 from django.utils.translation import gettext as _, gettext_lazy
 from django.views.decorators.cache import never_cache
+from django.views.decorators.common import no_append_slash
 from django.views.decorators.csrf import csrf_protect
 from django.views.i18n import JavaScriptCatalog
 
@@ -63,6 +67,8 @@ class AdminSite:
     password_change_template = None
     password_change_done_template = None
 
+    final_catch_all_view = True
+
     def __init__(self, name='admin'):
         self._registry = {}  # model_class class -> admin_class instance
         self.name = name
@@ -282,6 +288,10 @@ class AdminSite:
             urlpatterns += [
                 re_path(regex, wrap(self.app_index), name='app_list'),
             ]
+
+        if self.final_catch_all_view:
+            urlpatterns.append(re_path(r'(?P<url>.*)$', wrap(self.catch_all_view)))
+
         return urlpatterns
 
     @property
@@ -406,6 +416,20 @@ class AdminSite:
     def autocomplete_view(self, request):
         return AutocompleteJsonView.as_view(admin_site=self)(request)
 
+    @no_append_slash
+    def catch_all_view(self, request, url):
+        if settings.APPEND_SLASH and not url.endswith('/'):
+            urlconf = getattr(request, 'urlconf', None)
+            path = '%s/' % request.path_info
+            try:
+                match = resolve(path, urlconf)
+            except Resolver404:
+                pass
+            else:
+                if getattr(match.func, 'should_append_slash', True):
+                    return HttpResponsePermanentRedirect(path)
+        raise Http404
+
     def _build_app_dict(self, request, label=None):
         """
         Build the app dictionary. The optional `label` parameter filters models

docs/ref/contrib/admin/index.txt

@@ -2905,6 +2905,19 @@ Templates can override or extend base admin templates as described in
     A boolean value that determines whether to show the navigation sidebar
     on larger screens. By default, it is set to ``True``.
 
+.. attribute:: AdminSite.final_catch_all_view
+
+    .. versionadded:: 3.2
+
+    A boolean value that determines whether to add a final catch-all view to
+    the admin that redirects unauthenticated users to the login page. By
+    default, it is set to ``True``.
+
+    .. warning::
+
+        Setting this to ``False`` is not recommended as the view protects
+        against a potential model enumeration privacy issue.
+
 .. attribute:: AdminSite.login_template
 
     Path to a custom template that will be used by the admin site login view.

docs/releases/3.2.txt

@@ -125,6 +125,14 @@ Minor features
   <django.db.models.ForeignKey.limit_choices_to>` when searching a related
   model.
 
+* The admin now installs a final catch-all view that redirects unauthenticated
+  users to the login page, regardless or whether the URLs is otherwise valid.
+  This protects against a potential model enumeration privacy issue.
+
+  Although not recommended, you may set the new
+  :attr:`.AdminSite.final_catch_all_view` to ``False`` to disable the
+  catch-all view.
+
 :mod:`django.contrib.admindocs`
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
@@ -600,6 +608,12 @@ backends.
 * Pagination links in the admin are now 1-indexed instead of 0-indexed, i.e.
   the query string for the first page is ``?p=1`` instead of ``?p=0``.
 
+* The new admin catch-all view will break URL patterns routed after the admin
+  URLs and matching the admin URL prefix. You can either adjust your URL
+  ordering or, if necessary, set :attr:`AdminSite.final_catch_all_view
+  <django.contrib.admin.AdminSite.final_catch_all_view>` to ``False``,
+  disabling the catch-all view. See :ref:`whats-new-3.2` for more details.
+
 :mod:`django.contrib.gis`
 -------------------------
 

tests/admin_views/admin.py

@@ -15,10 +15,11 @@ from django.core.files.storage import FileSystemStorage
 from django.core.mail import EmailMessage
 from django.db import models
 from django.forms.models import BaseModelFormSet
-from django.http import HttpResponse, StreamingHttpResponse
+from django.http import HttpResponse, JsonResponse, StreamingHttpResponse
 from django.urls import path
 from django.utils.html import format_html
 from django.utils.safestring import mark_safe
+from django.views.decorators.common import no_append_slash
 
 from .forms import MediaActionForm
 from .models import (
@@ -100,7 +101,19 @@ class ArticleForm(forms.ModelForm):
         model = Article
 
 
-class ArticleAdmin(admin.ModelAdmin):
+class ArticleAdminWithExtraUrl(admin.ModelAdmin):
+    def get_urls(self):
+        urlpatterns = super().get_urls()
+        urlpatterns.append(
+            path('extra.json', self.admin_site.admin_view(self.extra_json), name='article_extra_json')
+        )
+        return urlpatterns
+
+    def extra_json(self, request):
+        return JsonResponse({})
+
+
+class ArticleAdmin(ArticleAdminWithExtraUrl):
     list_display = (
         'content', 'date', callable_year, 'model_year', 'modeladmin_year',
         'model_year_reversed', 'section', lambda obj: obj.title,
@@ -1181,5 +1194,19 @@ class ArticleAdmin9(admin.ModelAdmin):
         return obj is None
 
 
+class ActorAdmin9(admin.ModelAdmin):
+    def get_urls(self):
+        # Opt-out of append slash for single model.
+        urls = super().get_urls()
+        for pattern in urls:
+            pattern.callback = no_append_slash(pattern.callback)
+        return urls
+
+
 site9 = admin.AdminSite(name='admin9')
 site9.register(Article, ArticleAdmin9)
+site9.register(Actor, ActorAdmin9)
+
+site10 = admin.AdminSite(name='admin10')
+site10.final_catch_all_view = False
+site10.register(Article, ArticleAdminWithExtraUrl)

tests/admin_views/tests.py

@@ -6470,3 +6470,214 @@ class GetFormsetsWithInlinesArgumentTest(TestCase):
         post_data = {'name': '2'}
         response = self.client.post(reverse('admin:admin_views_implicitlygeneratedpk_change', args=(1,)), post_data)
         self.assertEqual(response.status_code, 302)
+
+
+@override_settings(ROOT_URLCONF='admin_views.urls')
+class AdminSiteFinalCatchAllPatternTests(TestCase):
+    """
+    Verifies the behaviour of the admin catch-all view.
+
+    * Anonynous/non-staff users are redirected to login for all URLs, whether
+      otherwise valid or not.
+    * APPEND_SLASH is applied for staff if needed.
+    * Otherwise Http404.
+    * Catch-all view disabled via AdminSite.final_catch_all_view.
+    """
+    def test_unknown_url_redirects_login_if_not_authenticated(self):
+        unknown_url = '/test_admin/admin/unknown/'
+        response = self.client.get(unknown_url)
+        self.assertRedirects(response, '%s?next=%s' % (reverse('admin:login'), unknown_url))
+
+    def test_unknown_url_404_if_authenticated(self):
+        superuser = User.objects.create_superuser(
+            username='super',
+            password='secret',
+            email='super@example.com',
+        )
+        self.client.force_login(superuser)
+        unknown_url = '/test_admin/admin/unknown/'
+        response = self.client.get(unknown_url)
+        self.assertEqual(response.status_code, 404)
+
+    def test_known_url_redirects_login_if_not_authenticated(self):
+        known_url = reverse('admin:admin_views_article_changelist')
+        response = self.client.get(known_url)
+        self.assertRedirects(response, '%s?next=%s' % (reverse('admin:login'), known_url))
+
+    def test_known_url_missing_slash_redirects_login_if_not_authenticated(self):
+        known_url = reverse('admin:admin_views_article_changelist')[:-1]
+        response = self.client.get(known_url)
+        # Redirects with the next URL also missing the slash.
+        self.assertRedirects(response, '%s?next=%s' % (reverse('admin:login'), known_url))
+
+    def test_non_admin_url_shares_url_prefix(self):
+        url = reverse('non_admin')[:-1]
+        response = self.client.get(url)
+        # Redirects with the next URL also missing the slash.
+        self.assertRedirects(response, '%s?next=%s' % (reverse('admin:login'), url))
+
+    def test_url_without_trailing_slash_if_not_authenticated(self):
+        url = reverse('admin:article_extra_json')
+        response = self.client.get(url)
+        self.assertRedirects(response, '%s?next=%s' % (reverse('admin:login'), url))
+
+    def test_unkown_url_without_trailing_slash_if_not_authenticated(self):
+        url = reverse('admin:article_extra_json')[:-1]
+        response = self.client.get(url)
+        self.assertRedirects(response, '%s?next=%s' % (reverse('admin:login'), url))
+
+    @override_settings(APPEND_SLASH=True)
+    def test_missing_slash_append_slash_true_unknown_url(self):
+        superuser = User.objects.create_user(
+            username='staff',
+            password='secret',
+            email='staff@example.com',
+            is_staff=True,
+        )
+        self.client.force_login(superuser)
+        unknown_url = '/test_admin/admin/unknown/'
+        response = self.client.get(unknown_url[:-1])
+        self.assertEqual(response.status_code, 404)
+
+    @override_settings(APPEND_SLASH=True)
+    def test_missing_slash_append_slash_true(self):
+        superuser = User.objects.create_user(
+            username='staff',
+            password='secret',
+            email='staff@example.com',
+            is_staff=True,
+        )
+        self.client.force_login(superuser)
+        known_url = reverse('admin:admin_views_article_changelist')
+        response = self.client.get(known_url[:-1])
+        self.assertRedirects(response, known_url, status_code=301, target_status_code=403)
+
+    @override_settings(APPEND_SLASH=True)
+    def test_missing_slash_append_slash_true_non_staff_user(self):
+        user = User.objects.create_user(
+            username='user',
+            password='secret',
+            email='user@example.com',
+            is_staff=False,
+        )
+        self.client.force_login(user)
+        known_url = reverse('admin:admin_views_article_changelist')
+        response = self.client.get(known_url[:-1])
+        self.assertRedirects(response, '/test_admin/admin/login/?next=/test_admin/admin/admin_views/article')
+
+    @override_settings(APPEND_SLASH=False)
+    def test_missing_slash_append_slash_false(self):
+        superuser = User.objects.create_user(
+            username='staff',
+            password='secret',
+            email='staff@example.com',
+            is_staff=True,
+        )
+        self.client.force_login(superuser)
+        known_url = reverse('admin:admin_views_article_changelist')
+        response = self.client.get(known_url[:-1])
+        self.assertEqual(response.status_code, 404)
+
+    @override_settings(APPEND_SLASH=True)
+    def test_single_model_no_append_slash(self):
+        superuser = User.objects.create_user(
+            username='staff',
+            password='secret',
+            email='staff@example.com',
+            is_staff=True,
+        )
+        self.client.force_login(superuser)
+        known_url = reverse('admin9:admin_views_actor_changelist')
+        response = self.client.get(known_url[:-1])
+        self.assertEqual(response.status_code, 404)
+
+    # Same tests above with final_catch_all_view=False.
+
+    def test_unknown_url_404_if_not_authenticated_without_final_catch_all_view(self):
+        unknown_url = '/test_admin/admin10/unknown/'
+        response = self.client.get(unknown_url)
+        self.assertEqual(response.status_code, 404)
+
+    def test_unknown_url_404_if_authenticated_without_final_catch_all_view(self):
+        superuser = User.objects.create_superuser(
+            username='super',
+            password='secret',
+            email='super@example.com',
+        )
+        self.client.force_login(superuser)
+        unknown_url = '/test_admin/admin10/unknown/'
+        response = self.client.get(unknown_url)
+        self.assertEqual(response.status_code, 404)
+
+    def test_known_url_redirects_login_if_not_authenticated_without_final_catch_all_view(self):
+        known_url = reverse('admin10:admin_views_article_changelist')
+        response = self.client.get(known_url)
+        self.assertRedirects(response, '%s?next=%s' % (reverse('admin10:login'), known_url))
+
+    def test_known_url_missing_slash_redirects_with_slash_if_not_authenticated_without_final_catch_all_view(self):
+        known_url = reverse('admin10:admin_views_article_changelist')
+        response = self.client.get(known_url[:-1])
+        self.assertRedirects(response, known_url, status_code=301, fetch_redirect_response=False)
+
+    def test_non_admin_url_shares_url_prefix_without_final_catch_all_view(self):
+        url = reverse('non_admin10')
+        response = self.client.get(url[:-1])
+        self.assertRedirects(response, url, status_code=301)
+
+    def test_url_without_trailing_slash_if_not_authenticated_without_final_catch_all_view(self):
+        url = reverse('admin10:article_extra_json')
+        response = self.client.get(url)
+        self.assertRedirects(response, '%s?next=%s' % (reverse('admin10:login'), url))
+
+    def test_unkown_url_without_trailing_slash_if_not_authenticated_without_final_catch_all_view(self):
+        url = reverse('admin10:article_extra_json')[:-1]
+        response = self.client.get(url)
+        # Matches test_admin/admin10/admin_views/article/<path:object_id>/
+        self.assertRedirects(response, url + '/', status_code=301, fetch_redirect_response=False)
+
+    @override_settings(APPEND_SLASH=True)
+    def test_missing_slash_append_slash_true_unknown_url_without_final_catch_all_view(self):
+        superuser = User.objects.create_user(
+            username='staff',
+            password='secret',
+            email='staff@example.com',
+            is_staff=True,
+        )
+        self.client.force_login(superuser)
+        unknown_url = '/test_admin/admin10/unknown/'
+        response = self.client.get(unknown_url[:-1])
+        self.assertEqual(response.status_code, 404)
+
+    @override_settings(APPEND_SLASH=True)
+    def test_missing_slash_append_slash_true_without_final_catch_all_view(self):
+        superuser = User.objects.create_user(
+            username='staff',
+            password='secret',
+            email='staff@example.com',
+            is_staff=True,
+        )
+        self.client.force_login(superuser)
+        known_url = reverse('admin10:admin_views_article_changelist')
+        response = self.client.get(known_url[:-1])
+        self.assertRedirects(response, known_url, status_code=301, target_status_code=403)
+
+    @override_settings(APPEND_SLASH=False)
+    def test_missing_slash_append_slash_false_without_final_catch_all_view(self):
+        superuser = User.objects.create_user(
+            username='staff',
+            password='secret',
+            email='staff@example.com',
+            is_staff=True,
+        )
+        self.client.force_login(superuser)
+        known_url = reverse('admin10:admin_views_article_changelist')
+        response = self.client.get(known_url[:-1])
+        self.assertEqual(response.status_code, 404)
+
+    # Outside admin.
+
+    def test_non_admin_url_404_if_not_authenticated(self):
+        unknown_url = '/unknown/'
+        response = self.client.get(unknown_url)
+        # Does not redirect to the admin login.
+        self.assertEqual(response.status_code, 404)

tests/admin_views/urls.py

@@ -1,8 +1,14 @@
+from django.http import HttpResponse
 from django.urls import include, path
 
 from . import admin, custom_has_permission_admin, customadmin, views
 from .test_autocomplete_view import site as autocomplete_site
 
+
+def non_admin_view(request):
+    return HttpResponse()
+
+
 urlpatterns = [
     path('test_admin/admin/doc/', include('django.contrib.admindocs.urls')),
     path('test_admin/admin/secure-view/', views.secure_view, name='secure_view'),
@@ -17,6 +23,10 @@ urlpatterns = [
     # All admin views accept `extra_context` to allow adding it like this:
     path('test_admin/admin8/', (admin.site.get_urls(), 'admin', 'admin-extra-context'), {'extra_context': {}}),
     path('test_admin/admin9/', admin.site9.urls),
+    path('test_admin/admin10/', admin.site10.urls),
     path('test_admin/has_permission_admin/', custom_has_permission_admin.site.urls),
     path('test_admin/autocomplete_admin/', autocomplete_site.urls),
+    # Shares the admin URL prefix.
+    path('test_admin/admin/non_admin_view/', non_admin_view, name='non_admin'),
+    path('test_admin/admin10/non_admin_view/', non_admin_view, name='non_admin10'),
 ]