diff --git a/netbox/netbox/authentication/__init__.py b/netbox/netbox/authentication/__init__.py index 19b1e58ca..164325379 100644 --- a/netbox/netbox/authentication/__init__.py +++ b/netbox/netbox/authentication/__init__.py @@ -1,13 +1,13 @@ import logging from collections import defaultdict +from django.apps import apps from django.conf import settings from django.contrib.auth.backends import ModelBackend from django.contrib.auth.backends import RemoteUserBackend as _RemoteUserBackend from django.contrib.auth.models import AnonymousUser from django.core.exceptions import ImproperlyConfigured from django.db.models import Q -from django.utils.translation import gettext_lazy as _ from users.constants import CONSTRAINT_TOKEN_USER from users.models import Group, ObjectPermission, User @@ -137,12 +137,18 @@ class ObjectPermissionMixin: if obj is None: return True - # Sanity check: Ensure that the requested permission applies to the specified object - model = obj._meta.concrete_model - if model._meta.label_lower != '.'.join((app_label, model_name)): - raise ValueError(_("Invalid permission {permission} for model {model}").format( - permission=perm, model=model - )) + # Sanity check: the permission must apply to the object's model. Permissions may name proxy + # models, so compare concrete models and evaluate constraints via the permission model's manager. + try: + permission_model = apps.get_model(app_label, model_name) + except LookupError: + logger = logging.getLogger('netbox.auth.ObjectPermissionBackend') + logger.warning(f"Permission {perm} does not reference a valid model") + return False + if permission_model._meta.concrete_model is not obj._meta.concrete_model: + logger = logging.getLogger('netbox.auth.ObjectPermissionBackend') + logger.debug(f"Permission {perm} is not valid for {obj._meta.label_lower} objects") + return False # Compile a QuerySet filter that matches all instances of the specified model tokens = { @@ -153,7 +159,7 @@ class ObjectPermissionMixin: # Permission to perform the requested action on the object depends on whether the specified object matches # the specified constraints. Note that this check is made against the *database* record representing the object, # not the instance itself. - return model.objects.filter(qs_filter, pk=obj.pk).exists() + return permission_model.objects.filter(qs_filter, pk=obj.pk).exists() class ObjectPermissionBackend(ObjectPermissionMixin, ModelBackend): diff --git a/netbox/netbox/tests/test_authentication.py b/netbox/netbox/tests/test_authentication.py index 5fda6e088..0c3f965d5 100644 --- a/netbox/netbox/tests/test_authentication.py +++ b/netbox/netbox/tests/test_authentication.py @@ -9,8 +9,10 @@ from django.urls import reverse from rest_framework.test import APIClient from social_core.exceptions import AuthFailed -from core.models import ObjectType +from core.choices import ManagedFileRootPathChoices +from core.models import ManagedFile, ObjectType from dcim.models import Rack, Site +from extras.models import ScriptModule from netbox.authentication.misc import _mirror_groups from netbox.middleware import SocialAuthExceptionMiddleware from users.constants import TOKEN_PREFIX @@ -745,6 +747,54 @@ class ObjectPermissionAPIViewTestCase(TestCase): self.assertEqual(response.status_code, 204) +class ObjectPermissionProxyModelTestCase(TestCase): + """ + Object-level permission checks against proxy models (e.g. extras.ScriptModule proxying + core.ManagedFile) must evaluate the permission rather than raise ValueError. + """ + @classmethod + def setUpTestData(cls): + cls.managed_file = ManagedFile.objects.create( + file_root=ManagedFileRootPathChoices.SCRIPTS, + file_path='proxy_permission_test.py' + ) + cls.script_module = ScriptModule.objects.get(pk=cls.managed_file.pk) + + def _grant_scriptmodule_permission(self, constraints=None): + obj_perm = ObjectPermission(name='ScriptModule change', actions=['change'], constraints=constraints) + obj_perm.save() + obj_perm.users.add(self.user) + obj_perm.object_types.add(ObjectType.objects.get_for_model(ScriptModule, for_concrete_model=False)) + + def test_has_perm_cross_app_proxy_model(self): + """An unconstrained proxy-model permission grants access to a proxy instance.""" + self._grant_scriptmodule_permission() + self.assertTrue(self.user.has_perm('extras.change_scriptmodule', self.script_module)) + + def test_has_perm_cross_app_proxy_model_matching_constraints(self): + """A constrained proxy-model permission grants access when the instance matches.""" + self._grant_scriptmodule_permission(constraints={'file_path': 'proxy_permission_test.py'}) + self.assertTrue(self.user.has_perm('extras.change_scriptmodule', self.script_module)) + + def test_has_perm_cross_app_proxy_model_nonmatching_constraints(self): + """A constrained proxy-model permission denies access when the instance does not match.""" + self._grant_scriptmodule_permission(constraints={'file_path': 'other.py'}) + self.assertFalse(self.user.has_perm('extras.change_scriptmodule', self.script_module)) + + def test_has_perm_invalid_permission_object_pair(self): + """A permission checked against an object of an unrelated model denies instead of raising.""" + site = Site.objects.create(name='Proxy Test Site', slug='proxy-test-site') + self._grant_scriptmodule_permission() + self.assertFalse(self.user.has_perm('extras.change_scriptmodule', site)) + + @override_settings(DEFAULT_PERMISSIONS={'extras.change_nosuchmodel': None}) + def test_has_perm_unknown_model_permission(self): + """A permission naming a nonexistent model denies and logs a warning instead of raising.""" + self._grant_scriptmodule_permission() + with self.assertLogs('netbox.auth.ObjectPermissionBackend', level='WARNING'): + self.assertFalse(self.user.has_perm('extras.change_nosuchmodel', self.script_module)) + + class SocialAuthExceptionMiddlewareTestCase(SimpleTestCase): """ Verify that SSO/SAML authentication failures are surfaced as a login-page message rather than