fix(ui): Suppress unauthorized embedded object tables

Add a `should_render()` hook to the `Panel` base class and override it
in `ObjectsTablePanel` to check the requesting user's view permission
for the panel's model. This prevents object detail pages from issuing
HTMX requests for related tables (e.g. locations, devices, image
attachments) that return 403 and disrupt the page.

Fixes #21893

Co-authored-by: Jeremy Stretch <jstretch@netboxlabs.com>

netbox/dcim/tests/test_views.py

@@ -194,6 +194,28 @@ class SiteTestCase(ViewTestCases.PrimaryObjectViewTestCase):
             'description': 'New description',
         }
 
+    def test_get_object_with_only_site_view_permission_hides_unauthorized_embedded_panels(self):
+        site = self._get_queryset().first()
+
+        obj_perm = ObjectPermission(
+            name='Test permission',
+            actions=['view'],
+        )
+        obj_perm.save()
+        obj_perm.users.add(self.user)
+        obj_perm.object_types.add(ObjectType.objects.get_for_model(self.model))
+
+        response = self.client.get(site.get_absolute_url())
+        self.assertHttpStatus(response, 200)
+
+        for panel, url in (
+            ('locations', reverse('dcim:location_list')),
+            ('devices', reverse('dcim:device_list')),
+            ('image attachments', reverse('extras:imageattachment_list')),
+        ):
+            with self.subTest(panel=panel):
+                self.assertNotContains(response, url)
+
 
 class LocationTestCase(ViewTestCases.OrganizationalObjectViewTestCase):
     model = Location

netbox/netbox/tests/test_ui.py

@@ -1,4 +1,4 @@
-from django.test import TestCase
+from django.test import RequestFactory, TestCase
 
 from circuits.choices import CircuitStatusChoices, VirtualCircuitTerminationRoleChoices
 from circuits.models import (
@@ -8,9 +8,12 @@ from circuits.models import (
     VirtualCircuitTermination,
     VirtualCircuitType,
 )
+from core.models import ObjectType
 from dcim.choices import InterfaceTypeChoices
-from dcim.models import Interface
+from dcim.models import Interface, Site
 from netbox.ui import attrs
+from netbox.ui.panels import ObjectsTablePanel
+from users.models import ObjectPermission, User
 from utilities.testing import create_test_device
 from vpn.choices import (
     AuthenticationAlgorithmChoices,
@@ -213,3 +216,55 @@ class RelatedObjectListAttrTest(TestCase):
         self.assertInHTML('<li>IKE Proposal 2</li>', rendered)
         self.assertNotIn('IKE Proposal 3', rendered)
         self.assertIn('…', rendered)
+
+
+class ObjectsTablePanelTest(TestCase):
+    """
+    Verify that ObjectsTablePanel.should_render() hides the panel when
+    the requesting user lacks view permission for the panel's model.
+    """
+
+    @classmethod
+    def setUpTestData(cls):
+        cls.user = User.objects.create_user(username='test_user', password='test_password')
+
+        # Grant view permission only for Site
+        obj_perm = ObjectPermission.objects.create(
+            name='View sites only',
+            actions=['view'],
+        )
+        obj_perm.object_types.add(ObjectType.objects.get_for_model(Site))
+        obj_perm.users.add(cls.user)
+
+    def setUp(self):
+        self.factory = RequestFactory()
+        self.panel = ObjectsTablePanel(model='dcim.site')
+        self.panel_no_perm = ObjectsTablePanel(model='dcim.location')
+
+    def _make_context(self, user=None):
+        if user is None:
+            return {}
+        request = self.factory.get('/')
+        request.user = user
+        return {'request': request}
+
+    def test_should_render_without_request(self):
+        """
+        Panel should render when no request is present in context.
+        """
+        context = self.panel.get_context({})
+        self.assertTrue(self.panel.should_render(context))
+
+    def test_should_render_with_permission(self):
+        """
+        Panel should render when the user has view permission for the panel's model.
+        """
+        context = self.panel.get_context(self._make_context(self.user))
+        self.assertTrue(self.panel.should_render(context))
+
+    def test_should_not_render_without_permission(self):
+        """
+        Panel should be hidden when the user lacks view permission for the panel's model.
+        """
+        context = self.panel_no_perm.get_context(self._make_context(self.user))
+        self.assertFalse(self.panel_no_perm.should_render(context))

netbox/netbox/ui/panels.py

@@ -5,6 +5,7 @@ from django.utils.translation import gettext_lazy as _
 from netbox.ui import attrs
 from netbox.ui.actions import CopyContent
 from utilities.data import resolve_attr_path
+from utilities.permissions import get_permission_for_model
 from utilities.querydict import dict_to_querydict
 from utilities.string import title
 from utilities.templatetags.plugins import _get_registered_content
@@ -74,6 +75,15 @@ class Panel:
             'panel_class': self.__class__.__name__,
         }
 
+    def should_render(self, context):
+        """
+        Determines whether the panel should render on the page. (Default: True)
+
+        Parameters:
+            context (dict): The panel's prepared context (the return value of get_context())
+        """
+        return True
+
     def render(self, context):
         """
         Render the panel as HTML.
@@ -81,7 +91,10 @@ class Panel:
         Parameters:
             context (dict): The template context
         """
-        return render_to_string(self.template_name, self.get_context(context))
+        ctx = self.get_context(context)
+        if not self.should_render(ctx):
+            return ''
+        return render_to_string(self.template_name, ctx, request=ctx.get('request'))
 
 
 #
@@ -314,6 +327,16 @@ class ObjectsTablePanel(Panel):
             'url_params': dict_to_querydict(url_params),
         }
 
+    def should_render(self, context):
+        """
+        Hide the panel if the user does not have view permission for the panel's model.
+        """
+        request = context.get('request')
+        if request is None:
+            return True
+
+        return request.user.has_perm(get_permission_for_model(self.model, 'view'))
+
 
 class TemplatePanel(Panel):
     """

netbox/tenancy/api/views.py

@@ -42,13 +42,7 @@ class TenantViewSet(NetBoxModelViewSet):
 #
 
 class ContactGroupViewSet(MPTTLockedMixin, NetBoxModelViewSet):
-    queryset = ContactGroup.objects.add_related_count(
-        ContactGroup.objects.all(),
-        Contact,
-        'groups',
-        'contact_count',
-        cumulative=True
-    )
+    queryset = ContactGroup.objects.annotate_contacts()
     serializer_class = serializers.ContactGroupSerializer
     filterset_class = filtersets.ContactGroupFilterSet
 

netbox/tenancy/models/contacts.py

@@ -1,12 +1,14 @@
 from django.contrib.contenttypes.fields import GenericForeignKey
 from django.core.exceptions import ValidationError
 from django.db import models
+from django.db.models.expressions import RawSQL
 from django.urls import reverse
 from django.utils.translation import gettext_lazy as _
 
 from netbox.models import ChangeLoggedModel, NestedGroupModel, OrganizationalModel, PrimaryModel
 from netbox.models.features import CustomFieldsMixin, ExportTemplatesMixin, TagsMixin, has_feature
 from tenancy.choices import *
+from utilities.mptt import TreeManager
 
 __all__ = (
     'Contact',
@@ -16,10 +18,34 @@ __all__ = (
 )
 
 
+class ContactGroupManager(TreeManager):
+
+    def annotate_contacts(self):
+        """
+        Annotate the total number of Contacts belonging to each ContactGroup.
+
+        This returns both direct children and children of child groups. Raw SQL is used here to avoid double-counting
+        contacts which are assigned to multiple child groups of the parent.
+        """
+        return self.annotate(
+            contact_count=RawSQL(
+                "SELECT COUNT(DISTINCT m2m.contact_id)"
+                " FROM tenancy_contact_groups m2m"
+                " INNER JOIN tenancy_contactgroup cg ON m2m.contactgroup_id = cg.id"
+                " WHERE cg.tree_id = tenancy_contactgroup.tree_id"
+                " AND cg.lft >= tenancy_contactgroup.lft"
+                " AND cg.lft <= tenancy_contactgroup.rght",
+                ()
+            )
+        )
+
+
 class ContactGroup(NestedGroupModel):
     """
     An arbitrary collection of Contacts.
     """
+    objects = ContactGroupManager()
+
     class Meta:
         ordering = ['name']
         # Empty tuple triggers Django migration detection for MPTT indexes

netbox/tenancy/tests/test_models.py

@@ -0,0 +1,72 @@
+from django.test import TestCase
+
+from tenancy.models import Contact, ContactGroup
+
+
+class ContactGroupTestCase(TestCase):
+
+    @classmethod
+    def setUpTestData(cls):
+        # Create a tree of contact groups:
+        #  - Group A
+        #    - Group A1
+        #    - Group A2
+        #  - Group B
+        cls.group_a = ContactGroup.objects.create(name='Group A', slug='group-a')
+        cls.group_a1 = ContactGroup.objects.create(name='Group A1', slug='group-a1', parent=cls.group_a)
+        cls.group_a2 = ContactGroup.objects.create(name='Group A2', slug='group-a2', parent=cls.group_a)
+        cls.group_b = ContactGroup.objects.create(name='Group B', slug='group-b')
+
+        # Create contacts
+        cls.contact1 = Contact.objects.create(name='Contact 1')
+        cls.contact2 = Contact.objects.create(name='Contact 2')
+        cls.contact3 = Contact.objects.create(name='Contact 3')
+        cls.contact4 = Contact.objects.create(name='Contact 4')
+
+    def test_annotate_contacts_direct(self):
+        """Contacts assigned directly to a group should be counted."""
+        self.contact1.groups.set([self.group_a])
+        self.contact2.groups.set([self.group_a])
+
+        queryset = ContactGroup.objects.annotate_contacts()
+        self.assertEqual(queryset.get(pk=self.group_a.pk).contact_count, 2)
+
+    def test_annotate_contacts_cumulative(self):
+        """Contacts assigned to child groups should be included in the parent's count."""
+        self.contact1.groups.set([self.group_a1])
+        self.contact2.groups.set([self.group_a2])
+
+        queryset = ContactGroup.objects.annotate_contacts()
+        self.assertEqual(queryset.get(pk=self.group_a.pk).contact_count, 2)
+        self.assertEqual(queryset.get(pk=self.group_a1.pk).contact_count, 1)
+        self.assertEqual(queryset.get(pk=self.group_a2.pk).contact_count, 1)
+
+    def test_annotate_contacts_no_double_counting(self):
+        """A contact assigned to multiple child groups must be counted only once for the parent."""
+        self.contact1.groups.set([self.group_a1, self.group_a2])
+
+        queryset = ContactGroup.objects.annotate_contacts()
+        self.assertEqual(queryset.get(pk=self.group_a.pk).contact_count, 1)
+
+    def test_annotate_contacts_mixed(self):
+        """Test a mix of direct and inherited contacts with overlap."""
+        self.contact1.groups.set([self.group_a])
+        self.contact2.groups.set([self.group_a1])
+        self.contact3.groups.set([self.group_a1, self.group_a2])
+        self.contact4.groups.set([self.group_b])
+
+        queryset = ContactGroup.objects.annotate_contacts()
+        # Group A: contact1 (direct) + contact2 (via A1) + contact3 (via A1 & A2) = 3
+        self.assertEqual(queryset.get(pk=self.group_a.pk).contact_count, 3)
+        # Group A1: contact2 + contact3 = 2
+        self.assertEqual(queryset.get(pk=self.group_a1.pk).contact_count, 2)
+        # Group A2: contact3 = 1
+        self.assertEqual(queryset.get(pk=self.group_a2.pk).contact_count, 1)
+        # Group B: contact4 = 1
+        self.assertEqual(queryset.get(pk=self.group_b.pk).contact_count, 1)
+
+    def test_annotate_contacts_empty(self):
+        """Groups with no contacts should return a count of zero."""
+        queryset = ContactGroup.objects.annotate_contacts()
+        self.assertEqual(queryset.get(pk=self.group_a.pk).contact_count, 0)
+        self.assertEqual(queryset.get(pk=self.group_b.pk).contact_count, 0)

netbox/tenancy/views.py

@@ -205,13 +205,7 @@ class TenantBulkDeleteView(generic.BulkDeleteView):
 
 @register_model_view(ContactGroup, 'list', path='', detail=False)
 class ContactGroupListView(generic.ObjectListView):
-    queryset = ContactGroup.objects.add_related_count(
-        ContactGroup.objects.all(),
-        Contact,
-        'groups',
-        'contact_count',
-        cumulative=True
-    )
+    queryset = ContactGroup.objects.annotate_contacts()
     filterset = filtersets.ContactGroupFilterSet
     filterset_form = forms.ContactGroupFilterForm
     table = tables.ContactGroupTable
@@ -254,7 +248,7 @@ class ContactGroupView(GetRelatedModelsMixin, generic.ObjectView):
                 request,
                 groups,
                 extra=(
-                    (Contact.objects.restrict(request.user, 'view').filter(groups__in=groups), 'group_id'),
+                    (Contact.objects.restrict(request.user, 'view').filter(groups__in=groups).distinct(), 'group_id'),
                 ),
             ),
         }
@@ -280,13 +274,7 @@ class ContactGroupBulkImportView(generic.BulkImportView):
 
 @register_model_view(ContactGroup, 'bulk_edit', path='edit', detail=False)
 class ContactGroupBulkEditView(generic.BulkEditView):
-    queryset = ContactGroup.objects.add_related_count(
-        ContactGroup.objects.all(),
-        Contact,
-        'groups',
-        'contact_count',
-        cumulative=True
-    )
+    queryset = ContactGroup.objects.annotate_contacts()
     filterset = filtersets.ContactGroupFilterSet
     table = tables.ContactGroupTable
     form = forms.ContactGroupBulkEditForm
@@ -300,13 +288,7 @@ class ContactGroupBulkRenameView(generic.BulkRenameView):
 
 @register_model_view(ContactGroup, 'bulk_delete', path='delete', detail=False)
 class ContactGroupBulkDeleteView(generic.BulkDeleteView):
-    queryset = ContactGroup.objects.add_related_count(
-        ContactGroup.objects.all(),
-        Contact,
-        'groups',
-        'contact_count',
-        cumulative=True
-    )
+    queryset = ContactGroup.objects.annotate_contacts()
     filterset = filtersets.ContactGroupFilterSet
     table = tables.ContactGroupTable