diff --git a/products/access_control/backend/facade/user_access_control.py b/products/access_control/backend/facade/user_access_control.py index db53e205adb5..29935d3cba3d 100644 --- a/products/access_control/backend/facade/user_access_control.py +++ b/products/access_control/backend/facade/user_access_control.py @@ -1125,8 +1125,8 @@ def filter_queryset_by_access_level( # Apply filtering logic based on resource-level access if not self.has_resource_access(resource) and decision.allowed_ids: - # User has "none" resource access but specific object access - # Only show objects they have explicit access to (plus created objects) + # User has "none" resource access but some objects' own rules grant access. + # Only show those objects (plus objects they created) if model_has_creator: queryset = queryset.filter(Q(id__in=decision.allowed_ids) | Q(created_by=self._user)) else: @@ -1146,7 +1146,8 @@ def _blocked_and_allowed_object_ids(self, access_controls: list[_AccessControl]) Explicit-wins: if a resource_id has any explicit (role/member) rule, the object is allowed when any explicit rule grants non-"none", otherwise blocked. With no explicit - rule, the object is blocked only when every default rule is "none". + rule, the default ("everyone in the project") rules decide the same way: any non-"none" + default rule allows the object, all-"none" blocks it. Reads the `role_id` / `organization_member_id` columns rather than the `.role` / `.organization_member` FK accessors — equivalent result (id is None iff the relation is @@ -1171,7 +1172,8 @@ def _blocked_and_allowed_object_ids(self, access_controls: list[_AccessControl]) if not explicit_access_controls: if all(access_level == NO_ACCESS_LEVEL for access_level in access_levels): blocked_resource_ids.add(resource_id) - # No explicit controls for this object - don't block it + else: + allowed_resource_ids.add(resource_id) continue # Check if user has any non-"none" access to this specific object @@ -1221,8 +1223,8 @@ def allowlisted_resource_ids_by_scope(self) -> dict[APIScopeObject, frozenset[st where they hold object-level grants but no resource-level access at all. This is the allowlist branch of `filter_queryset_by_access_level`: with "none" at the - resource level, REST serves the route and narrows rows to the explicitly granted objects - instead of merely removing denied ones. HogQL consumers must narrow the same way — a + resource level, REST serves the route and narrows rows to the objects whose own rules + grant access instead of merely removing denied ones. HogQL consumers must narrow the same way — a resource absent from this mapping falls back to removing `blocked_resource_ids_by_scope`. Empty for org admins and when there is no team / EE / entitlement, matching diff --git a/products/access_control/backend/tests/test_user_access_control.py b/products/access_control/backend/tests/test_user_access_control.py index 2f3393b95382..4932fe56d31e 100644 --- a/products/access_control/backend/tests/test_user_access_control.py +++ b/products/access_control/backend/tests/test_user_access_control.py @@ -1286,6 +1286,40 @@ def test_filter_queryset_by_access_level_with_none_resource_and_specific_access( assert self.notebook_3.id in notebook_ids # Created by user assert self.notebook_2.id not in notebook_ids # No access + def test_filter_queryset_allowlists_objects_granted_to_everyone(self): + from products.notebooks.backend.models import Notebook + + # Resource-level "none", but notebook_1 is shared with everyone in the project + self._create_access_control(resource="notebook", access_level="none") + self._create_access_control(resource="notebook", resource_id=str(self.notebook_1.id), access_level="editor") + + self._clear_uac_caches() + + notebook_ids = list( + self.user_access_control.filter_queryset_by_access_level(Notebook.objects.all()).values_list( + "id", flat=True + ) + ) + assert self.notebook_1.id in notebook_ids # Granted to everyone on the object + assert self.notebook_3.id in notebook_ids # Created by user + assert self.notebook_2.id not in notebook_ids + + # A member-level "none" row on the same object overrides the everyone grant + self._create_access_control( + resource="notebook", + resource_id=str(self.notebook_1.id), + access_level="none", + organization_member=self.organization_membership, + ) + self._clear_uac_caches() + + notebook_ids = list( + self.user_access_control.filter_queryset_by_access_level(Notebook.objects.all()).values_list( + "id", flat=True + ) + ) + assert self.notebook_1.id not in notebook_ids + def test_filter_queryset_by_access_level_with_resource_access(self): """Test queryset filtering when user has resource-level access""" from products.notebooks.backend.models import Notebook diff --git a/products/access_control/backend/tests/test_user_access_control_pbt.py b/products/access_control/backend/tests/test_user_access_control_pbt.py index bcb945ad973c..d4b8bd45bea9 100644 --- a/products/access_control/backend/tests/test_user_access_control_pbt.py +++ b/products/access_control/backend/tests/test_user_access_control_pbt.py @@ -432,8 +432,8 @@ def oracle_blocked_and_allowed_object_ids( ) -> tuple[set[str], set[str]]: # Mirrors _blocked_and_allowed_object_ids over the rows visible to self.user # (only MATCHING targets survive _filter_options). Explicit (role/member) rows - # decide an object: any non-"none" explicit row allows it, otherwise it's blocked. - # With no explicit row, the object is blocked only when every default row is "none". + # decide an object when present, otherwise the default rows decide: any + # non-"none" deciding row allows the object, all-"none" blocks it. blocked: set[str] = set() allowed: set[str] = set() for resource_id, specs in object_specs_by_id.items(): @@ -441,11 +441,8 @@ def oracle_blocked_and_allowed_object_ids( if not matching: continue explicit = [s for s in matching if s.target != "team_default"] - if not explicit: - if all(s.level == NO_ACCESS_LEVEL for s in matching): - blocked.add(resource_id) - continue - if any(s.level != NO_ACCESS_LEVEL for s in explicit): + deciding = explicit or matching + if any(s.level != NO_ACCESS_LEVEL for s in deciding): allowed.add(resource_id) else: blocked.add(resource_id)