From c6dfdf10e5938e8ee7af18e31663023fb4b8390d Mon Sep 17 00:00:00 2001 From: jeremystretch Date: Fri, 1 Jul 2022 11:38:39 -0400 Subject: [PATCH] Introduce qs_filter_from_constraints() for constructing object permission QS filters --- netbox/netbox/authentication.py | 22 +++++++++------------- netbox/utilities/permissions.py | 24 ++++++++++++++++++++++++ netbox/utilities/querysets.py | 27 +++++++-------------------- 3 files changed, 40 insertions(+), 33 deletions(-) diff --git a/netbox/netbox/authentication.py b/netbox/netbox/authentication.py index 00fb3ee66..c16095fdc 100644 --- a/netbox/netbox/authentication.py +++ b/netbox/netbox/authentication.py @@ -9,7 +9,9 @@ from django.core.exceptions import ImproperlyConfigured from django.db.models import Q from users.models import ObjectPermission -from utilities.permissions import permission_is_exempt, resolve_permission, resolve_permission_ct +from utilities.permissions import ( + permission_is_exempt, qs_filter_from_constraints, resolve_permission, resolve_permission_ct, +) UserModel = get_user_model() @@ -99,8 +101,10 @@ class ObjectPermissionMixin: if not user_obj.is_active or user_obj.is_anonymous: return False + object_permissions = self.get_all_permissions(user_obj) + # If no applicable ObjectPermissions have been created for this user/permission, deny permission - if perm not in self.get_all_permissions(user_obj): + if perm not in object_permissions: return False # If no object has been specified, grant permission. (The presence of a permission in this set tells @@ -113,21 +117,13 @@ class ObjectPermissionMixin: if model._meta.label_lower != '.'.join((app_label, model_name)): raise ValueError(f"Invalid permission {perm} for model {model}") - # Compile a query filter that matches all instances of the specified model - obj_perm_constraints = self.get_all_permissions(user_obj)[perm] - constraints = Q() - for perm_constraints in obj_perm_constraints: - if perm_constraints: - constraints |= Q(**perm_constraints) - else: - # Found ObjectPermission with null constraints; allow model-level access - constraints = Q() - break + # Compile a QuerySet filter that matches all instances of the specified model + qs_filter = qs_filter_from_constraints(object_permissions[perm]) # 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(constraints, pk=obj.pk).exists() + return model.objects.filter(qs_filter, pk=obj.pk).exists() class ObjectPermissionBackend(ObjectPermissionMixin, ModelBackend): diff --git a/netbox/utilities/permissions.py b/netbox/utilities/permissions.py index b11bf504a..123df9e45 100644 --- a/netbox/utilities/permissions.py +++ b/netbox/utilities/permissions.py @@ -1,5 +1,14 @@ from django.conf import settings from django.contrib.contenttypes.models import ContentType +from django.db.models import Q + +__all__ = ( + 'get_permission_for_model', + 'permission_is_exempt', + 'qs_filter_from_constraints', + 'resolve_permission', + 'resolve_permission_ct', +) def get_permission_for_model(model, action): @@ -69,3 +78,18 @@ def permission_is_exempt(name): return True return False + + +def qs_filter_from_constraints(constraints): + """ + Construct a Q filter object from an iterable of ObjectPermission constraints. + """ + params = Q() + for constraint in constraints: + if constraint: + params |= Q(**constraint) + else: + # Found null constraint; permit model-level access + return Q() + + return params diff --git a/netbox/utilities/querysets.py b/netbox/utilities/querysets.py index 97d2e8779..8ec6012bf 100644 --- a/netbox/utilities/querysets.py +++ b/netbox/utilities/querysets.py @@ -1,6 +1,6 @@ -from django.db.models import Q, QuerySet +from django.db.models import QuerySet -from utilities.permissions import permission_is_exempt +from utilities.permissions import permission_is_exempt, qs_filter_from_constraints class RestrictedQuerySet(QuerySet): @@ -28,23 +28,10 @@ class RestrictedQuerySet(QuerySet): # Filter the queryset to include only objects with allowed attributes else: - attrs = Q() - for perm_attrs in user._object_perm_cache[permission_required]: - if type(perm_attrs) is list: - for p in perm_attrs: - attrs |= Q(**p) - elif perm_attrs: - attrs |= Q(**perm_attrs) - else: - # Any permission with null constraints grants access to _all_ instances - attrs = Q() - break - else: - # for else, when no break - # avoid duplicates when JOIN on many-to-many fields without using DISTINCT. - # DISTINCT acts globally on the entire request, which may not be desirable. - allowed_objects = self.model.objects.filter(attrs) - attrs = Q(pk__in=allowed_objects) - qs = self.filter(attrs) + attrs = qs_filter_from_constraints(user._object_perm_cache[permission_required]) + # #8715: Avoid duplicates when JOIN on many-to-many fields without using DISTINCT. + # DISTINCT acts globally on the entire request, which may not be desirable. + allowed_objects = self.model.objects.filter(attrs) + qs = self.filter(pk__in=allowed_objects) return qs