Просмотр исходного кода

Fixes #22507: Check is_active in restrict() and IsSuperuser superuser bypass (#22508)

RestrictedQuerySet.restrict() and IsSuperuser short-circuited on is_superuser
without checking is_active, so a deactivated superuser was granted the full
superuser bypass. restrict() in particular fails open, returning the
unrestricted queryset. Both now also require is_active, matching the existing
guard in ObjectPermissionMixin.has_perm.
Jason Novinger 1 неделя назад
Родитель
Сommit
8c73f46cc9
3 измененных файлов с 28 добавлено и 2 удалено
  1. 26 0
      netbox/users/tests/test_models.py
  2. 1 1
      netbox/utilities/api.py
  3. 1 1
      netbox/utilities/querysets.py

+ 26 - 0
netbox/users/tests/test_models.py

@@ -232,3 +232,29 @@ class UserConfigTestCase(TestCase):
 
         # Clear a non-existing value; should fail silently
         userconfig.clear('invalid')
+
+
+class RestrictedQuerySetTestCase(TestCase):
+    """
+    Test the is_active handling of RestrictedQuerySet.restrict()'s superuser bypass.
+    """
+
+    @classmethod
+    def setUpTestData(cls):
+        # Token uses a plain RestrictedQuerySet manager, so its objects() exercises restrict()
+        # directly without requiring any object permissions to be configured.
+        cls.token = Token.objects.create(user=create_test_user('Token Owner'))
+
+    def test_active_superuser_bypasses_restriction(self):
+        user = User.objects.create(username='active_su', is_superuser=True, is_active=True)
+        self.assertIn(self.token, Token.objects.restrict(user, 'view'))
+
+    def test_inactive_superuser_does_not_bypass_restriction(self):
+        """
+        A deactivated superuser must not bypass restrict(). Without an explicit
+        view permission they receive an empty queryset, mirroring the is_active
+        guard in ObjectPermissionMixin.has_perm.
+        """
+        user = User.objects.create(username='inactive_su', is_superuser=True, is_active=False)
+        self.assertNotIn(self.token, Token.objects.restrict(user, 'view'))
+        self.assertEqual(Token.objects.restrict(user, 'view').count(), 0)

+ 1 - 1
netbox/utilities/api.py

@@ -44,7 +44,7 @@ class IsSuperuser(BasePermission):
     Allows access only to superusers.
     """
     def has_permission(self, request, view):
-        return bool(request.user and request.user.is_superuser)
+        return bool(request.user and request.user.is_active and request.user.is_superuser)
 
 
 def get_serializer_for_model(model, prefix=''):

+ 1 - 1
netbox/utilities/querysets.py

@@ -49,7 +49,7 @@ class RestrictedQuerySet(QuerySet):
         permission_required = get_permission_for_model(self.model, action)
 
         # Bypass restriction for superusers and exempt views
-        if user and user.is_superuser or permission_is_exempt(permission_required):
+        if (user and user.is_active and user.is_superuser) or permission_is_exempt(permission_required):
             return self
 
         # User is anonymous or has not been granted the requisite permission