Browse Source

Fixes #23235: Restrict Jobs tab and Script Job views to viewable jobs (#23405)

The object Jobs tab, the script Jobs tab and job count, and the script
list's last run all built Job querysets without restricting them to the
requesting user, so a constrained core.view_job permission was ignored.
Jason Novinger 21 giờ trước cách đây
mục cha
commit
9023ed4972

+ 59 - 1
netbox/core/tests/test_views.py

@@ -16,7 +16,7 @@ from rq.registry import DeferredJobRegistry, FailedJobRegistry, FinishedJobRegis
 from core.choices import ObjectChangeActionChoices
 from core.models import *
 from dcim.models import Site
-from users.models import User
+from users.models import ObjectPermission, User
 from utilities.testing import TestCase, ViewTestCases, create_tags, disable_logging
 from utilities.testing.mixins import RQQueueTestMixin
 
@@ -171,6 +171,64 @@ class JobTestCase(
         )
 
 
+class ObjectJobsViewTestCase(TestCase):
+    """
+    The Jobs tab of an object must list only the Jobs which the user is permitted to view.
+    """
+    user_permissions = (
+        'core.view_datasource',
+    )
+
+    @classmethod
+    def setUpTestData(cls):
+        cls.datasource = DataSource.objects.create(
+            name='Data Source 1',
+            type='local',
+            source_url='file:///var/tmp/source1/',
+        )
+        cls.other_user = User.objects.create_user(username='otheruser')
+        cls.other_job = cls._create_job('other-job', user=cls.other_user)
+        cls.system_job = cls._create_job('system-job', user=None)
+
+    @classmethod
+    def _create_job(cls, name, user):
+        return Job.objects.create(
+            name=name,
+            object_type=ContentType.objects.get_for_model(DataSource),
+            object_id=cls.datasource.pk,
+            status='completed',
+            job_id=uuid.uuid4(),
+            user=user,
+        )
+
+    def setUp(self):
+        super().setUp()
+        self.own_job = self._create_job('own-job', user=self.user)
+        self.url = reverse('core:datasource_jobs', kwargs={'pk': self.datasource.pk})
+
+    def _grant_view_job(self, constraints=None):
+        obj_perm = ObjectPermission(name='View jobs', constraints=constraints, actions=['view'])
+        obj_perm.save()
+        obj_perm.users.add(self.user)
+        obj_perm.object_types.add(ObjectType.objects.get_for_model(Job))
+
+    def _get_listed_jobs(self):
+        response = self.client.get(self.url)
+        self.assertHttpStatus(response, 200)
+        return {row.record for row in response.context['table'].rows}
+
+    def test_unconstrained_permission_lists_all_jobs(self):
+        self._grant_view_job()
+        self.assertEqual(self._get_listed_jobs(), {self.own_job, self.other_job, self.system_job})
+
+    def test_user_constrained_permission_lists_own_jobs_only(self):
+        self._grant_view_job(constraints={'user': '$user'})
+        self.assertEqual(self._get_listed_jobs(), {self.own_job})
+
+    def test_without_view_job_permission_lists_no_jobs(self):
+        self.assertEqual(self._get_listed_jobs(), set())
+
+
 class JobLogViewTestCase(TestCase):
     user_permissions = (
         'core.view_job',

+ 100 - 0
netbox/extras/tests/test_views.py

@@ -1,3 +1,4 @@
+import datetime
 import hashlib
 import logging
 import uuid
@@ -1733,3 +1734,102 @@ class ScriptResultViewTestCase(TestCase):
         response = self.client.get(url, {'export': 'output'})
         self.assertHttpStatus(response, 404)
         self.assertNotIn(self.SECRET_OUTPUT.encode(), response.content)
+
+
+class ScriptJobsRestrictionTestCase(TestCase):
+    """
+    Script views must display only the Jobs which the user is permitted to view.
+    """
+    # ScriptModule is a proxy model, so viewing it is governed by its concrete model's permission
+    user_permissions = ('extras.view_script', 'core.view_managedfile')
+
+    class TestScriptClass(PythonClass):
+        class Meta:
+            name = 'Restriction test'
+
+        def run(self, data, commit):
+            return 'Complete'
+
+    @classmethod
+    def setUpTestData(cls):
+        with patch.object(ScriptModule, 'sync_classes'):
+            module = ScriptModule.objects.create(
+                file_root=ManagedFileRootPathChoices.SCRIPTS,
+                file_path='restriction_script.py',
+            )
+        cls.script = Script.objects.create(module=module, name='Restriction test', is_executable=True)
+        cls.other_user = User.objects.create_user(username='otheruser')
+
+        # The other user's Job is the most recent, so that the script list must skip it (rather than
+        # hide it) to find the most recent Job the user is permitted to view
+        cls.system_job = cls._create_job('system-job', user=None, minutes_ago=30)
+        cls.other_job = cls._create_job('other-job', user=cls.other_user, minutes_ago=10)
+
+    @classmethod
+    def _create_job(cls, name, user, minutes_ago):
+        started = now() - datetime.timedelta(minutes=minutes_ago)
+        return Job.objects.create(
+            name=name,
+            object_type=ObjectType.objects.get_for_model(Script),
+            object_id=cls.script.pk,
+            status=JobStatusChoices.STATUS_COMPLETED,
+            job_id=uuid.uuid4(),
+            user=user,
+            started=started,
+            completed=started,
+        )
+
+    def setUp(self):
+        super().setUp()
+        python_class_patch = patch.object(Script, 'python_class', new=self.TestScriptClass)
+        python_class_patch.start()
+        self.addCleanup(python_class_patch.stop)
+        self.own_job = self._create_job('own-job', user=self.user, minutes_ago=20)
+
+    def _grant_view_job(self, constraints=None):
+        obj_perm = ObjectPermission(name='View jobs', constraints=constraints, actions=['view'])
+        obj_perm.save()
+        obj_perm.users.add(self.user)
+        obj_perm.object_types.add(ObjectType.objects.get_for_model(Job))
+
+    def _assert_script_views_show(self, expected_jobs):
+        """
+        Assert that the script's Jobs table and job count reflect only the expected Jobs.
+        """
+        response = self.client.get(reverse('extras:script_jobs', kwargs={'pk': self.script.pk}))
+        self.assertHttpStatus(response, 200)
+        self.assertEqual({row.record for row in response.context['table'].rows}, expected_jobs)
+        self.assertEqual(response.context['job_count'], len(expected_jobs))
+
+        for view_name in ('extras:script', 'extras:script_source'):
+            response = self.client.get(reverse(view_name, kwargs={'pk': self.script.pk}))
+            self.assertHttpStatus(response, 200)
+            self.assertEqual(response.context['job_count'], len(expected_jobs))
+
+    def _assert_script_list_shows(self, expected_job):
+        """
+        Assert that the script list displays the expected Job (or none) as the script's most recent result.
+        """
+        response = self.client.get(reverse('extras:script_list'))
+        self.assertHttpStatus(response, 200)
+        content = response.content.decode()
+        for job in (self.own_job, self.other_job, self.system_job):
+            result_url = reverse('extras:script_result', kwargs={'job_pk': job.pk})
+            if job == expected_job:
+                self.assertIn(result_url, content)
+            else:
+                self.assertNotIn(result_url, content)
+
+    def test_unconstrained_permission(self):
+        self._grant_view_job()
+        self._assert_script_views_show({self.own_job, self.other_job, self.system_job})
+        self._assert_script_list_shows(self.other_job)
+
+    def test_user_constrained_permission(self):
+        self._grant_view_job(constraints={'user': '$user'})
+        self._assert_script_views_show({self.own_job})
+        self._assert_script_list_shows(self.own_job)
+
+    def test_without_view_job_permission(self):
+        self._assert_script_views_show(set())
+        self._assert_script_list_shows(None)

+ 13 - 5
netbox/extras/views.py

@@ -1672,10 +1672,17 @@ class ScriptListView(ContentTypePermissionRequiredMixin, View):
             'data_source', 'data_file',
         )
 
+        # Find the most recent completed Job for each Script which the user is permitted to view
+        latest_jobs = {
+            script.pk: script.get_latest_jobs().restrict(request.user, 'view').first()
+            for script in available_scripts
+        }
+
         context = {
             'model': ScriptModule,
             'script_modules': script_modules,
             'available_scripts': available_scripts,
+            'latest_jobs': latest_jobs,
         }
 
         # Use partial template for dashboard widgets
@@ -1724,7 +1731,7 @@ class ScriptView(BaseScriptView):
             'script': script,
             'script_class': script_class,
             'form': form,
-            'job_count': script.jobs.count(),
+            'job_count': script.jobs.restrict(request.user, 'view').count(),
         })
 
     def post(self, request, **kwargs):
@@ -1781,7 +1788,7 @@ class ScriptView(BaseScriptView):
             'script': script,
             'script_class': script.python_class(),
             'form': form,
-            'job_count': script.jobs.count(),
+            'job_count': script.jobs.restrict(request.user, 'view').count(),
         })
 
 
@@ -1795,7 +1802,7 @@ class ScriptSourceView(BaseScriptView):
         return render(request, 'extras/script/source.html', {
             'script': script,
             'script_class': script_class,
-            'job_count': script.jobs.count(),
+            'job_count': script.jobs.restrict(request.user, 'view').count(),
             'tab': 'source',
         })
 
@@ -1806,13 +1813,14 @@ class ScriptJobsView(BaseScriptView):
     def get(self, request, **kwargs):
         script = self.get_object(**kwargs)
 
-        jobs_table = ScriptJobTable(data=script.jobs.all(), orderable=False)
+        jobs = script.jobs.restrict(request.user, 'view')
+        jobs_table = ScriptJobTable(data=jobs, orderable=False)
         jobs_table.configure(request)
 
         return render(request, 'extras/script/jobs.html', {
             'script': script,
             'table': jobs_table,
-            'job_count': script.jobs.count(),
+            'job_count': jobs.count(),
             'tab': 'jobs',
         })
 

+ 2 - 2
netbox/netbox/views/generic/feature_views.py

@@ -202,8 +202,8 @@ class ObjectJobsView(ConditionalLoginRequiredMixin, View):
         self.model = model
         obj = self.get_object(request, **kwargs)
 
-        # Gather all Jobs for this object
-        jobs = self.get_jobs(obj)
+        # Gather all Jobs for this object which the user is permitted to view
+        jobs = self.get_jobs(obj).restrict(request.user, 'view')
         jobs_table = JobTable(data=jobs, orderable=False)
         jobs_table.configure(request)
 

+ 1 - 1
netbox/templates/extras/inc/script_list_content.html

@@ -39,7 +39,7 @@
           <tbody>
             {% for script in scripts %}
               {% if script in available_scripts %}
-                {% with last_job=script.get_latest_jobs|first %}
+                {% with last_job=latest_jobs|get_key:script.pk %}
                   <tr>
                     <td>
                       {% if script.is_executable %}