diff --git a/ee/api/rbac/access_control.py b/ee/api/rbac/access_control.py index 877989358ffe2..6173ccee28e06 100644 --- a/ee/api/rbac/access_control.py +++ b/ee/api/rbac/access_control.py @@ -79,25 +79,6 @@ def validate(self, data): return data -class AccessControlFilterBackend(BaseFilterBackend): - """ - Filters the queryset automatically based on access controls - """ - - def filter_queryset( - self, - request: Request, - queryset: Union[QuerySet, RawQuerySet], - view: APIView, - ): - if isinstance(queryset, RawQuerySet): - return queryset - - queryset = view.user_access_control.filter_queryset_by_access_level(queryset) - - return queryset - - class AccessControlViewSetMixin: """ Adds an "access_controls" action to the viewset that handles access control for the given resource @@ -110,7 +91,17 @@ class AccessControlViewSetMixin: # 2. Get the actual object which we can pass to the serializer to check if the user created it # 3. We can also use the serializer to check the access level for the object - filter_backends = [AccessControlFilterBackend] + def filter_queryset(self, queryset): + queryset = super().filter_queryset(queryset) + # TODO: Detect GET param to include hidden resources (for admins) + + if self.action != "list": + # NOTE: If we are getting an individual object then we don't filter it out here - this is handled by the permission logic + # The reason being, that if we filter out here already, we can't load the object which is required for checking access controls for it + return queryset + queryset = self.user_access_control.filter_queryset_by_access_level(queryset) + + return queryset def _get_access_control_serializer(self, *args, **kwargs): kwargs.setdefault("context", self.get_serializer_context()) diff --git a/posthog/api/feature_flag.py b/posthog/api/feature_flag.py index 2d2c8179b3f15..2132e71da14a0 100644 --- a/posthog/api/feature_flag.py +++ b/posthog/api/feature_flag.py @@ -363,9 +363,9 @@ class Meta: class FeatureFlagViewSet( TeamAndOrgViewSetMixin, + AccessControlViewSetMixin, TaggedItemViewSetMixin, ForbidDestroyModel, - AccessControlViewSetMixin, viewsets.ModelViewSet, ): """ diff --git a/posthog/api/insight.py b/posthog/api/insight.py index d15974c006739..32253c69ea4ea 100644 --- a/posthog/api/insight.py +++ b/posthog/api/insight.py @@ -562,9 +562,9 @@ def dashboard_tile_from_context(self, insight: Insight, dashboard: Optional[Dash class InsightViewSet( TeamAndOrgViewSetMixin, + AccessControlViewSetMixin, TaggedItemViewSetMixin, ForbidDestroyModel, - AccessControlViewSetMixin, viewsets.ModelViewSet, ): scope_object = "insight" diff --git a/posthog/api/notebook.py b/posthog/api/notebook.py index a0dd84c359b6c..e0d56314523b8 100644 --- a/posthog/api/notebook.py +++ b/posthog/api/notebook.py @@ -237,7 +237,7 @@ def update(self, instance: Notebook, validated_data: Dict, **kwargs) -> Notebook ], ) ) -class NotebookViewSet(TeamAndOrgViewSetMixin, ForbidDestroyModel, viewsets.ModelViewSet, AccessControlViewSetMixin): +class NotebookViewSet(TeamAndOrgViewSetMixin, AccessControlViewSetMixin, ForbidDestroyModel, viewsets.ModelViewSet): scope_object = "notebook" queryset = Notebook.objects.all() filter_backends = [DjangoFilterBackend] diff --git a/posthog/api/routing.py b/posthog/api/routing.py index 5ab0930f59a88..393d62bb88d70 100644 --- a/posthog/api/routing.py +++ b/posthog/api/routing.py @@ -244,11 +244,7 @@ def user_permissions(self) -> "UserPermissions": def user_access_control(self) -> "UserAccessControl": team: Optional[Team] = None try: - # TRICKY: We don't want to load the team if that is the object we are validating (because we get into a recursive loop) - if not self.request.resolver_match.url_name.startswith("team-"): - # TODO: This calls get_object for team.py which triggers a recursive loop via get_queryset (which uses this method) - # It's mostly a problem for team view because the object we are filtering for _is_ this object :see_no_evil: - team = self.team + team = self.team except (Team.DoesNotExist, KeyError): pass diff --git a/posthog/rbac/user_access_control.py b/posthog/rbac/user_access_control.py index 9a0ec7f611430..e1f7dc7023f66 100644 --- a/posthog/rbac/user_access_control.py +++ b/posthog/rbac/user_access_control.py @@ -201,7 +201,9 @@ def filter_queryset_by_access_level(self, queryset: QuerySet) -> QuerySet: # Find all items related to the queryset model that have access controls such that the effective level for the user is "none" # and exclude them from the queryset - resource = model_to_resource(queryset.model) + model = queryset.model + resource = model_to_resource(model) + model_has_creator = hasattr(model, "created_by") # TODO: Make this more efficient role_memberships = self._user.role_memberships.select_related("role").all() @@ -242,13 +244,13 @@ def filter_queryset_by_access_level(self, queryset: QuerySet) -> QuerySet: if all(access_level == NO_ACCESS_LEVEL for access_level in access_levels): blocked_resource_ids.add(resource_id) - # TODO: Don't filter the queryset if the user is an admin - # TODO: Allow the creator of the resource to be able to see it - could either do this by saving - # a special access control when adding one if no other exists, or by saving a link to the foreign resource - # Filter the queryset based on the access controls if blocked_resource_ids: - queryset = queryset.exclude(id__in=blocked_resource_ids) + # Filter out any IDs where the user is not the creator and the id is blocked + if model_has_creator: + queryset = queryset.exclude(Q(id__in=blocked_resource_ids) & ~Q(created_by=self._user)) + else: + queryset = queryset.exclude(id__in=blocked_resource_ids) return queryset