Closes #16137: Remove is_staff boolean from User model (#20306)

* Closes #16137: Remove is_staff boolean from User model

* Remove default is_staff value from UserManager.create_user()

* Restore staff_only on MenuItem

* Introduce IsSuperuser API permission to replace IsAdminUser

* Update and improve RQ task API view tests

* Remove is_staff attribute assignment from RemoteUserBackend

netbox/core/api/views.py

@@ -9,7 +9,6 @@ from drf_spectacular.utils import extend_schema
 from rest_framework import viewsets
 from rest_framework.decorators import action
 from rest_framework.exceptions import PermissionDenied
-from rest_framework.permissions import IsAdminUser
 from rest_framework.response import Response
 from rest_framework.routers import APIRootView
 from rest_framework.viewsets import ReadOnlyModelViewSet
@@ -24,6 +23,7 @@ from netbox.api.authentication import IsAuthenticatedOrLoginNotRequired
 from netbox.api.metadata import ContentTypeMetadata
 from netbox.api.pagination import LimitOffsetListPagination
 from netbox.api.viewsets import NetBoxModelViewSet, NetBoxReadOnlyModelViewSet
+from utilities.api import IsSuperuser
 from . import serializers
 
 
@@ -99,7 +99,7 @@ class BaseRQViewSet(viewsets.ViewSet):
     """
     Base class for RQ view sets. Provides a list() method. Subclasses must implement get_data().
     """
-    permission_classes = [IsAdminUser]
+    permission_classes = [IsSuperuser]
     serializer_class = None
 
     def get_data(self):

netbox/core/tests/test_api.py

@@ -134,10 +134,7 @@ class BackgroundTaskTestCase(TestCase):
         Create a user and token for API calls.
         """
         # Create the test user and assign permissions
-        self.user = User.objects.create_user(username='testuser')
-        self.user.is_staff = True
-        self.user.is_active = True
-        self.user.save()
+        self.user = User.objects.create_user(username='testuser', is_active=True)
         self.token = Token.objects.create(user=self.user)
         self.header = {'HTTP_AUTHORIZATION': f'Token {self.token.key}'}
 
@@ -150,13 +147,11 @@ class BackgroundTaskTestCase(TestCase):
         url = reverse('core-api:rqqueue-list')
 
         # Attempt to load view without permission
-        self.user.is_staff = False
-        self.user.save()
         response = self.client.get(url, **self.header)
         self.assertEqual(response.status_code, 403)
 
         # Load view with permission
-        self.user.is_staff = True
+        self.user.is_superuser = True
         self.user.save()
         response = self.client.get(url, **self.header)
         self.assertEqual(response.status_code, 200)
@@ -165,7 +160,16 @@ class BackgroundTaskTestCase(TestCase):
         self.assertIn('low', str(response.content))
 
     def test_background_queue(self):
-        response = self.client.get(reverse('core-api:rqqueue-detail', args=['default']), **self.header)
+        url = reverse('core-api:rqqueue-detail', args=['default'])
+
+        # Attempt to load view without permission
+        response = self.client.get(url, **self.header)
+        self.assertEqual(response.status_code, 403)
+
+        # Load view with permission
+        self.user.is_superuser = True
+        self.user.save()
+        response = self.client.get(url, **self.header)
         self.assertEqual(response.status_code, 200)
         self.assertIn('default', str(response.content))
         self.assertIn('oldest_job_timestamp', str(response.content))
@@ -174,8 +178,16 @@ class BackgroundTaskTestCase(TestCase):
     def test_background_task_list(self):
         queue = get_queue('default')
         queue.enqueue(self.dummy_job_default)
+        url = reverse('core-api:rqtask-list')
 
-        response = self.client.get(reverse('core-api:rqtask-list'), **self.header)
+        # Attempt to load view without permission
+        response = self.client.get(url, **self.header)
+        self.assertEqual(response.status_code, 403)
+
+        # Load view with permission
+        self.user.is_superuser = True
+        self.user.save()
+        response = self.client.get(url, **self.header)
         self.assertEqual(response.status_code, 200)
         self.assertIn('origin', str(response.content))
         self.assertIn('core.tests.test_api.BackgroundTaskTestCase.dummy_job_default()', str(response.content))
@@ -183,8 +195,16 @@ class BackgroundTaskTestCase(TestCase):
     def test_background_task(self):
         queue = get_queue('default')
         job = queue.enqueue(self.dummy_job_default)
+        url = reverse('core-api:rqtask-detail', args=[job.id])
 
-        response = self.client.get(reverse('core-api:rqtask-detail', args=[job.id]), **self.header)
+        # Attempt to load view without permission
+        response = self.client.get(url, **self.header)
+        self.assertEqual(response.status_code, 403)
+
+        # Load view with permission
+        self.user.is_superuser = True
+        self.user.save()
+        response = self.client.get(url, **self.header)
         self.assertEqual(response.status_code, 200)
         self.assertIn(str(job.id), str(response.content))
         self.assertIn('origin', str(response.content))
@@ -194,45 +214,65 @@ class BackgroundTaskTestCase(TestCase):
     def test_background_task_delete(self):
         queue = get_queue('default')
         job = queue.enqueue(self.dummy_job_default)
+        url = reverse('core-api:rqtask-delete', args=[job.id])
 
-        response = self.client.post(reverse('core-api:rqtask-delete', args=[job.id]), **self.header)
+        # Attempt to load view without permission
+        response = self.client.get(url, **self.header)
+        self.assertEqual(response.status_code, 403)
+
+        # Load view with permission
+        self.user.is_superuser = True
+        self.user.save()
+        response = self.client.post(url, **self.header)
         self.assertEqual(response.status_code, 200)
         self.assertFalse(RQ_Job.exists(job.id, connection=queue.connection))
         queue = get_queue('default')
         self.assertNotIn(job.id, queue.job_ids)
 
     def test_background_task_requeue(self):
-        queue = get_queue('default')
-
         # Enqueue & run a job that will fail
+        queue = get_queue('default')
         job = queue.enqueue(self.dummy_job_failing)
         worker = get_worker('default')
         with disable_logging():
             worker.work(burst=True)
         self.assertTrue(job.is_failed)
+        url = reverse('core-api:rqtask-requeue', args=[job.id])
+
+        # Attempt to requeue the job without permission
+        response = self.client.post(url, **self.header)
+        self.assertEqual(response.status_code, 403)
 
         # Re-enqueue the failed job and check that its status has been reset
-        response = self.client.post(reverse('core-api:rqtask-requeue', args=[job.id]), **self.header)
+        self.user.is_superuser = True
+        self.user.save()
+        response = self.client.post(url, **self.header)
         self.assertEqual(response.status_code, 200)
         job = RQ_Job.fetch(job.id, queue.connection)
         self.assertFalse(job.is_failed)
 
     def test_background_task_enqueue(self):
-        queue = get_queue('default')
-
         # Enqueue some jobs that each depends on its predecessor
+        queue = get_queue('default')
         job = previous_job = None
         for _ in range(0, 3):
             job = queue.enqueue(self.dummy_job_default, depends_on=previous_job)
             previous_job = job
+        url = reverse('core-api:rqtask-enqueue', args=[job.id])
 
         # Check that the last job to be enqueued has a status of deferred
         self.assertIsNotNone(job)
         self.assertEqual(job.get_status(), JobStatus.DEFERRED)
         self.assertIsNone(job.enqueued_at)
 
+        # Attempt to force-enqueue the job without permission
+        response = self.client.post(url, **self.header)
+        self.assertEqual(response.status_code, 403)
+
         # Force-enqueue the deferred job
-        response = self.client.post(reverse('core-api:rqtask-enqueue', args=[job.id]), **self.header)
+        self.user.is_superuser = True
+        self.user.save()
+        response = self.client.post(url, **self.header)
         self.assertEqual(response.status_code, 200)
 
         # Check that job's status is updated correctly
@@ -242,19 +282,27 @@ class BackgroundTaskTestCase(TestCase):
 
     def test_background_task_stop(self):
         queue = get_queue('default')
-
         worker = get_worker('default')
         job = queue.enqueue(self.dummy_job_default)
         worker.prepare_job_execution(job)
-
+        url = reverse('core-api:rqtask-stop', args=[job.id])
         self.assertEqual(job.get_status(), JobStatus.STARTED)
-        response = self.client.post(reverse('core-api:rqtask-stop', args=[job.id]), **self.header)
+
+        # Attempt to stop the task without permission
+        response = self.client.post(url, **self.header)
+        self.assertEqual(response.status_code, 403)
+
+        # Stop the task
+        self.user.is_superuser = True
+        self.user.save()
+        response = self.client.post(url, **self.header)
         self.assertEqual(response.status_code, 200)
         with disable_logging():
             worker.monitor_work_horse(job, queue)  # Sets the job as Failed and removes from Started
         started_job_registry = StartedJobRegistry(queue.name, connection=queue.connection)
         self.assertEqual(len(started_job_registry), 0)
 
+        # Verify that the task was cancelled
         canceled_job_registry = FailedJobRegistry(queue.name, connection=queue.connection)
         self.assertEqual(len(canceled_job_registry), 1)
         self.assertIn(job.id, canceled_job_registry)
@@ -262,19 +310,34 @@ class BackgroundTaskTestCase(TestCase):
     def test_worker_list(self):
         worker1 = get_worker('default', name=uuid.uuid4().hex)
         worker1.register_birth()
-
         worker2 = get_worker('high')
         worker2.register_birth()
+        url = reverse('core-api:rqworker-list')
 
-        response = self.client.get(reverse('core-api:rqworker-list'), **self.header)
+        # Attempt to fetch the worker list without permission
+        response = self.client.get(url, **self.header)
+        self.assertEqual(response.status_code, 403)
+
+        # Fetch the worker list
+        self.user.is_superuser = True
+        self.user.save()
+        response = self.client.get(url, **self.header)
         self.assertEqual(response.status_code, 200)
         self.assertIn(str(worker1.name), str(response.content))
 
     def test_worker(self):
         worker1 = get_worker('default', name=uuid.uuid4().hex)
         worker1.register_birth()
+        url = reverse('core-api:rqworker-detail', args=[worker1.name])
 
-        response = self.client.get(reverse('core-api:rqworker-detail', args=[worker1.name]), **self.header)
+        # Attempt to fetch a worker without permission
+        response = self.client.get(url, **self.header)
+        self.assertEqual(response.status_code, 403)
+
+        # Fetch the worker
+        self.user.is_superuser = True
+        self.user.save()
+        response = self.client.get(url, **self.header)
         self.assertEqual(response.status_code, 200)
         self.assertIn(str(worker1.name), str(response.content))
         self.assertIn('birth_date', str(response.content))

netbox/core/tests/test_views.py

@@ -158,7 +158,7 @@ class BackgroundTaskTestCase(TestCase):
 
     def setUp(self):
         super().setUp()
-        self.user.is_staff = True
+        self.user.is_superuser = True
         self.user.is_active = True
         self.user.save()
 
@@ -171,13 +171,13 @@ class BackgroundTaskTestCase(TestCase):
         url = reverse('core:background_queue_list')
 
         # Attempt to load view without permission
-        self.user.is_staff = False
+        self.user.is_superuser = False
         self.user.save()
         response = self.client.get(url)
         self.assertEqual(response.status_code, 403)
 
         # Load view with permission
-        self.user.is_staff = True
+        self.user.is_superuser = True
         self.user.save()
         response = self.client.get(url)
         self.assertEqual(response.status_code, 200)
@@ -356,7 +356,7 @@ class SystemTestCase(TestCase):
     def setUp(self):
         super().setUp()
 
-        self.user.is_staff = True
+        self.user.is_superuser = True
         self.user.save()
 
     def test_system_view_default(self):

netbox/core/views.py

@@ -366,7 +366,7 @@ class ConfigRevisionRestoreView(ContentTypePermissionRequiredMixin, View):
 class BaseRQView(UserPassesTestMixin, View):
 
     def test_func(self):
-        return self.request.user.is_staff
+        return self.request.user.is_superuser
 
 
 class BackgroundQueueListView(TableMixin, BaseRQView):
@@ -549,7 +549,7 @@ class WorkerView(BaseRQView):
 class SystemView(UserPassesTestMixin, View):
 
     def test_func(self):
-        return self.request.user.is_staff
+        return self.request.user.is_superuser
 
     def get(self, request):
 
@@ -632,7 +632,7 @@ class BasePluginView(UserPassesTestMixin, View):
     CACHE_KEY_CATALOG_ERROR = 'plugins-catalog-error'
 
     def test_func(self):
-        return self.request.user.is_staff
+        return self.request.user.is_superuser
 
     def get_cached_plugins(self, request):
         catalog_plugins = {}