tighten /admin/ access to superusers only

Author: dogboat

dojo/admin.py

@@ -14,6 +14,20 @@
     TextQuestion,
 )
 
+
+# Django's default admin gate is `is_active and is_staff`. Under the legacy
+# OS auth model is_staff is already a near-superuser bypass for queryset
+# filters and most permission checks, so the standard UserAdmin change form
+# would let any is_staff user with auth.change_user tick is_superuser on
+# themselves. Rather than narrowing each ModelAdmin individually, restrict
+# the entire admin site to superusers — there is no DefectDojo OS role that
+# legitimately needs /admin/ access without being a superuser.
+def _admin_site_has_permission(request):
+    return request.user.is_active and request.user.is_superuser
+
+
+admin.site.has_permission = _admin_site_has_permission
+
 # Conditionally unregister LogEntry from auditlog if it's registered
 try:
     from auditlog.models import LogEntry

unittests/test_adminsite.py

@@ -1,5 +1,7 @@
 import django.apps
 from django.contrib import admin
+from django.contrib.auth import get_user_model
+from django.urls import reverse
 
 from .dojo_test_case import DojoTestCase
 
@@ -20,3 +22,43 @@ def test_is_model_defined(self):
                 else:
                     with self.subTest(type="tag", subclass=subclass):
                         self.assertIn(subclass, admin.site._registry.keys(), f"{subclass} is not registered in 'tagulous.admin' in models.py")
+
+
+class AdminAccessGate(DojoTestCase):
+
+    """
+    is_staff is a near-superuser bypass under the legacy OS auth model, so
+    /admin/ must require is_superuser. Django's default UserAdmin change
+    form would otherwise let any is_staff user with auth.change_user tick
+    is_superuser on themselves.
+    """
+
+    def test_staff_non_superuser_blocked_from_admin(self):
+        User = get_user_model()
+        staff = User.objects.create_user(
+            username="staff-no-root", password="x", is_staff=True,
+        )
+        self.client.force_login(staff)
+
+        for url in (
+            reverse("admin:index"),
+            reverse("admin:auth_user_changelist"),
+            reverse("admin:auth_user_change", args=[staff.id]),
+        ):
+            with self.subTest(url=url):
+                resp = self.client.get(url)
+                self.assertEqual(resp.status_code, 302, url)
+                self.assertIn("/admin/login/", resp["Location"], url)
+
+    def test_superuser_can_reach_admin(self):
+        User = get_user_model()
+        root = User.objects.create_superuser(
+            username="root-test", email="r@example.com", password="x",
+        )
+        self.client.force_login(root)
+
+        self.assertEqual(self.client.get(reverse("admin:index")).status_code, 200)
+        self.assertEqual(
+            self.client.get(reverse("admin:auth_user_changelist")).status_code,
+            200,
+        )