fix(permissioning): Fix restricted projects being visible (#19218)

* Add tests to diagnose the issue

* Add THE test

* Implement the fix

* Update query snapshots

* Update query snapshots

* Ignore mypy's ignorance

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

ee/api/test/test_organization.py

@@ -274,3 +274,20 @@ class TestOrganizationEnterpriseAPI(APILicensedTest):
             self.organization.refresh_from_db()
             self.assertFalse(self.organization.is_feature_available("whatever"))
         License.PLANS = current_plans
+
+    def test_get_organization_restricted_teams_hidden(self):
+        self.organization_membership.level = OrganizationMembership.Level.MEMBER
+        self.organization_membership.save()
+        Team.objects.create(
+            organization=self.organization,
+            name="FORBIDDEN",
+            access_control=True,
+        )
+
+        response = self.client.get(f"/api/organizations/{self.organization.id}")
+
+        self.assertEqual(response.status_code, 200)
+        self.assertListEqual(
+            [team["name"] for team in response.json()["teams"]],
+            [self.team.name],  # "FORBIDDEN" excluded
+        )

ee/clickhouse/models/test/__snapshots__/test_property.ambr

@@ -57,7 +57,7 @@
 ---
 # name: test_parse_groups_persons_edge_case_with_single_filter
   <class 'tuple'> (
-    'AND (  has(%(vglobalperson_0)s, "pmat_email"))',
+    'AND (  has(%(vglobalperson_0)s, replaceRegexpAll(JSONExtractRaw(person_props, %(kglobalperson_0)s), \'^"|"$\', \'\')))',
     <class 'dict'> {
       'kglobalperson_0': 'email',
       'vglobalperson_0': <class 'list'> [

ee/clickhouse/views/test/__snapshots__/test_clickhouse_experiment_secondary_results.ambr

@@ -1,6 +1,6 @@
 # name: ClickhouseTestExperimentSecondaryResults.test_basic_secondary_metric_results
   '
-  /* user_id:131 celery:posthog.celery.sync_insight_caching_state */
+  /* user_id:132 celery:posthog.celery.sync_insight_caching_state */
   SELECT team_id,
          date_diff('second', max(timestamp), now()) AS age
   FROM events

posthog/api/organization.py

@@ -114,12 +114,8 @@ class OrganizationSerializer(serializers.ModelSerializer, UserPermissionsSeriali
         return membership.level if membership is not None else None
 
     def get_teams(self, instance: Organization) -> List[Dict[str, Any]]:
-        teams = cast(
-            List[Dict[str, Any]],
-            TeamBasicSerializer(instance.teams.all(), context=self.context, many=True).data,
-        )
-        visible_team_ids = set(self.user_permissions.team_ids_visible_for_user)
-        return [team for team in teams if team["id"] in visible_team_ids]
+        visible_teams = instance.teams.filter(id__in=self.user_permissions.team_ids_visible_for_user)
+        return TeamBasicSerializer(visible_teams, context=self.context, many=True).data  # type: ignore
 
     def get_metadata(self, instance: Organization) -> Dict[str, Union[str, int, object]]:
         return {

posthog/test/test_user_permissions.py

@@ -45,6 +45,22 @@ class TestUserTeamPermissions(BaseTest, WithPermissionsBase):
         with self.assertNumQueries(1):
             assert permissions.team(self.team).effective_membership_level is None
 
+    def test_team_effective_membership_level_membership_isolation(self):
+        self.team.access_control = True
+        self.team.save()
+        ExplicitTeamMembership.objects.create(
+            team=self.team,
+            parent_membership=self.organization_membership,
+        )
+        forbidden_team = Team.objects.create(
+            organization=self.organization,
+            name="FORBIDDEN",
+            access_control=True,
+        )
+        permissions = UserPermissions(user=self.user)
+        with self.assertNumQueries(2):
+            assert permissions.team(forbidden_team).effective_membership_level is None
+
     def test_team_effective_membership_level_with_explicit_membership_returns_current_level(self):
         self.team.access_control = True
         self.team.save()

posthog/user_permissions.py

@@ -72,7 +72,6 @@ class UserPermissions:
         candidate_teams = Team.objects.filter(organization_id__in=self.organizations.keys()).only(
             "pk", "organization_id", "access_control"
         )
-
         return [team.pk for team in candidate_teams if self.team(team).effective_membership_level is not None]
 
     # Cached properties/functions for efficient lookups in other classes
@@ -96,7 +95,7 @@ class UserPermissions:
         return {membership.organization_id: membership for membership in memberships}
 
     @cached_property
-    def explicit_team_memberships(self) -> Dict[UUID, Any]:
+    def explicit_team_memberships(self) -> Dict[int, Any]:
         try:
             from ee.models import ExplicitTeamMembership
         except ImportError:
@@ -104,8 +103,8 @@ class UserPermissions:
 
         memberships = ExplicitTeamMembership.objects.filter(
             parent_membership_id__in=[membership.pk for membership in self.organization_memberships.values()]
-        ).only("parent_membership_id", "level")
-        return {membership.parent_membership_id: membership.level for membership in memberships}
+        ).only("parent_membership_id", "level", "team_id")
+        return {membership.team_id: membership.level for membership in memberships}
 
     @cached_property
     def dashboard_privileges(self) -> Dict[int, Dashboard.PrivilegeLevel]:
@@ -168,7 +167,7 @@ class UserTeamPermissions:
         ):
             return organization_membership.level
 
-        explicit_membership_level = self.p.explicit_team_memberships.get(organization_membership.pk)
+        explicit_membership_level = self.p.explicit_team_memberships.get(self.team.id)
         if explicit_membership_level is not None:
             return max(explicit_membership_level, organization_membership.level)
         # Only organizations admins and above get implicit project membership