diff --git a/ee/api/feature_flag_role_access.py b/ee/api/feature_flag_role_access.py index 6d03c7a4f361c..01aa98a05b9db 100644 --- a/ee/api/feature_flag_role_access.py +++ b/ee/api/feature_flag_role_access.py @@ -1,10 +1,10 @@ from rest_framework import exceptions, mixins, serializers, viewsets from rest_framework.permissions import SAFE_METHODS, BasePermission -from ee.api.role import RoleSerializer +from ee.api.rbac.role import RoleSerializer from ee.models.feature_flag_role_access import FeatureFlagRoleAccess -from ee.models.organization_resource_access import OrganizationResourceAccess -from ee.models.role import Role +from ee.models.rbac.organization_resource_access import OrganizationResourceAccess +from ee.models.rbac.role import Role from posthog.api.feature_flag import FeatureFlagSerializer from posthog.api.routing import TeamAndOrgViewSetMixin from posthog.models import FeatureFlag diff --git a/ee/api/rbac/access_control.py b/ee/api/rbac/access_control.py new file mode 100644 index 0000000000000..9d84323b299a2 --- /dev/null +++ b/ee/api/rbac/access_control.py @@ -0,0 +1,192 @@ +from typing import TYPE_CHECKING, cast + + +from rest_framework import exceptions, serializers, status +from rest_framework.decorators import action +from rest_framework.request import Request +from rest_framework.response import Response +from rest_framework.viewsets import GenericViewSet + +from ee.models.rbac.access_control import AccessControl +from posthog.models.scopes import API_SCOPE_OBJECTS, APIScopeObjectOrNotSupported +from posthog.models.team.team import Team +from posthog.rbac.user_access_control import ( + ACCESS_CONTROL_LEVELS_RESOURCE, + UserAccessControl, + default_access_level, + highest_access_level, + ordered_access_levels, +) + + +if TYPE_CHECKING: + _GenericViewSet = GenericViewSet +else: + _GenericViewSet = object + + +class AccessControlSerializer(serializers.ModelSerializer): + access_level = serializers.CharField(allow_null=True) + + class Meta: + model = AccessControl + fields = [ + "resource", + "resource_id", + "access_level", + "organization_member", + "role", + "created_by", + "created_at", + "updated_at", + ] + read_only_fields = ["id", "created_at", "created_by"] + + def validate_resource(self, resource): + if resource not in API_SCOPE_OBJECTS: + raise serializers.ValidationError("Invalid resource. Must be one of: {}".format(API_SCOPE_OBJECTS)) + + return resource + + # validate that access control is a valid option + def validate_access_level(self, access_level): + if access_level and access_level not in ordered_access_levels(self.initial_data["resource"]): + raise serializers.ValidationError( + f"Invalid access level. Must be one of: {', '.join(ordered_access_levels(self.initial_data['resource']))}" + ) + + return access_level + + def validate(self, data): + context = self.context + # Ensure that only one of organization_member or role is set + if data.get("organization_member") and data.get("role"): + raise serializers.ValidationError("You can not scope an access control to both a member and a role.") + + access_control = cast(UserAccessControl, self.context["view"].user_access_control) + resource = data["resource"] + resource_id = data.get("resource_id") + + # We assume the highest level is required for the given resource to edit access controls + required_level = highest_access_level(resource) + team = context["view"].team + the_object = context["view"].get_object() + + if resource_id: + # Check that they have the right access level for this specific resource object + if not access_control.check_can_modify_access_levels_for_object(the_object): + raise exceptions.PermissionDenied(f"Must be {required_level} to modify {resource} permissions.") + else: + # If modifying the base resource rules then we are checking the parent membership (project or organization) + # NOTE: Currently we only support org level in the UI so its simply an org level check + if not access_control.check_can_modify_access_levels_for_object(team): + raise exceptions.PermissionDenied("Must be an Organization admin to modify project-wide permissions.") + + return data + + +class AccessControlViewSetMixin(_GenericViewSet): + """ + Adds an "access_controls" action to the viewset that handles access control for the given resource + + Why a mixin? We want to easily add this to any existing resource, including providing easy helpers for adding access control info such + as the current users access level to any response. + """ + + # 1. Know that the project level access is covered by the Permission check + # 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 + + def _get_access_control_serializer(self, *args, **kwargs): + kwargs.setdefault("context", self.get_serializer_context()) + return AccessControlSerializer(*args, **kwargs) + + def _get_access_controls(self, request: Request, is_global=False): + resource = cast(APIScopeObjectOrNotSupported, getattr(self, "scope_object", None)) + user_access_control = cast(UserAccessControl, self.user_access_control) # type: ignore + team = cast(Team, self.team) # type: ignore + + if is_global and resource != "project" or not resource or resource == "INTERNAL": + raise exceptions.NotFound("Role based access controls are only available for projects.") + + obj = self.get_object() + resource_id = obj.id + + if is_global: + # If role based then we are getting all controls for the project that aren't specific to a resource + access_controls = AccessControl.objects.filter(team=team, resource_id=None).all() + else: + # Otherwise we are getting all controls for the specific resource + access_controls = AccessControl.objects.filter(team=team, resource=resource, resource_id=resource_id).all() + + serializer = self._get_access_control_serializer(instance=access_controls, many=True) + user_access_level = user_access_control.access_level_for_object(obj, resource) + + return Response( + { + "access_controls": serializer.data, + # NOTE: For Role based controls we are always configuring resource level items + "available_access_levels": ACCESS_CONTROL_LEVELS_RESOURCE + if is_global + else ordered_access_levels(resource), + "default_access_level": "editor" if is_global else default_access_level(resource), + "user_access_level": user_access_level, + "user_can_edit_access_levels": user_access_control.check_can_modify_access_levels_for_object(obj), + } + ) + + def _update_access_controls(self, request: Request, is_global=False): + resource = getattr(self, "scope_object", None) + obj = self.get_object() + resource_id = str(obj.id) + team = cast(Team, self.team) # type: ignore + + # Generically validate the incoming data + if not is_global: + # If not role based we are deriving from the viewset + data = request.data + data["resource"] = resource + data["resource_id"] = resource_id + + partial_serializer = self._get_access_control_serializer(data=request.data) + partial_serializer.is_valid(raise_exception=True) + params = partial_serializer.validated_data + + instance = AccessControl.objects.filter( + team=team, + resource=params["resource"], + resource_id=params.get("resource_id"), + organization_member=params.get("organization_member"), + role=params.get("role"), + ).first() + + if params["access_level"] is None: + if instance: + instance.delete() + return Response(status=status.HTTP_204_NO_CONTENT) + + # Perform the upsert + if instance: + serializer = self._get_access_control_serializer(instance, data=request.data) + else: + serializer = self._get_access_control_serializer(data=request.data) + + serializer.is_valid(raise_exception=True) + serializer.validated_data["team"] = team + serializer.save() + + return Response(serializer.data, status=status.HTTP_200_OK) + + @action(methods=["GET", "PUT"], detail=True) + def access_controls(self, request: Request, *args, **kwargs): + if request.method == "PUT": + return self._update_access_controls(request) + + return self._get_access_controls(request) + + @action(methods=["GET", "PUT"], detail=True) + def global_access_controls(self, request: Request, *args, **kwargs): + if request.method == "PUT": + return self._update_access_controls(request, is_global=True) + + return self._get_access_controls(request, is_global=True) diff --git a/ee/api/organization_resource_access.py b/ee/api/rbac/organization_resource_access.py similarity index 92% rename from ee/api/organization_resource_access.py rename to ee/api/rbac/organization_resource_access.py index bf886566605b5..2d16c9bee90cf 100644 --- a/ee/api/organization_resource_access.py +++ b/ee/api/rbac/organization_resource_access.py @@ -1,7 +1,7 @@ from rest_framework import mixins, serializers, viewsets +from ee.api.rbac.role import RolePermissions -from ee.api.role import RolePermissions -from ee.models.organization_resource_access import OrganizationResourceAccess +from ee.models.rbac.organization_resource_access import OrganizationResourceAccess from posthog.api.routing import TeamAndOrgViewSetMixin diff --git a/ee/api/role.py b/ee/api/rbac/role.py similarity index 81% rename from ee/api/role.py rename to ee/api/rbac/role.py index 96041cd0109ef..b8120be2a8547 100644 --- a/ee/api/role.py +++ b/ee/api/rbac/role.py @@ -4,15 +4,15 @@ from rest_framework import mixins, serializers, viewsets from rest_framework.permissions import SAFE_METHODS, BasePermission -from ee.models.feature_flag_role_access import FeatureFlagRoleAccess -from ee.models.organization_resource_access import OrganizationResourceAccess -from ee.models.role import Role, RoleMembership +from ee.models.rbac.organization_resource_access import OrganizationResourceAccess +from ee.models.rbac.role import Role, RoleMembership from posthog.api.organization_member import OrganizationMemberSerializer from posthog.api.routing import TeamAndOrgViewSetMixin from posthog.api.shared import UserBasicSerializer +from posthog.constants import AvailableFeature from posthog.models import OrganizationMembership -from posthog.models.feature_flag import FeatureFlag from posthog.models.user import User +from posthog.permissions import PremiumFeaturePermission class RolePermissions(BasePermission): @@ -38,7 +38,6 @@ def has_permission(self, request, view): class RoleSerializer(serializers.ModelSerializer): created_by = UserBasicSerializer(read_only=True) members = serializers.SerializerMethodField() - associated_flags = serializers.SerializerMethodField() class Meta: model = Role @@ -49,7 +48,6 @@ class Meta: "created_at", "created_by", "members", - "associated_flags", ] read_only_fields = ["id", "created_at", "created_by"] @@ -75,29 +73,13 @@ def get_members(self, role: Role): members = RoleMembership.objects.filter(role=role) return RoleMembershipSerializer(members, many=True).data - def get_associated_flags(self, role: Role): - associated_flags: list[dict] = [] - role_access_objects = FeatureFlagRoleAccess.objects.filter(role=role).values_list("feature_flag_id") - flags = FeatureFlag.objects.filter(id__in=role_access_objects) - for flag in flags: - associated_flags.append({"id": flag.id, "key": flag.key}) - return associated_flags - - -class RoleViewSet( - TeamAndOrgViewSetMixin, - mixins.ListModelMixin, - mixins.CreateModelMixin, - mixins.RetrieveModelMixin, - mixins.UpdateModelMixin, - mixins.DestroyModelMixin, - viewsets.GenericViewSet, -): +class RoleViewSet(TeamAndOrgViewSetMixin, viewsets.ModelViewSet): scope_object = "organization" - permission_classes = [RolePermissions] serializer_class = RoleSerializer queryset = Role.objects.all() + permission_classes = [RolePermissions, PremiumFeaturePermission] + premium_feature = AvailableFeature.ROLE_BASED_ACCESS def safely_get_queryset(self, queryset): return queryset.filter(**self.request.GET.dict()) @@ -139,7 +121,8 @@ class RoleMembershipViewSet( viewsets.GenericViewSet, ): scope_object = "organization" - permission_classes = [RolePermissions] + permission_classes = [RolePermissions, PremiumFeaturePermission] + premium_feature = AvailableFeature.ROLE_BASED_ACCESS serializer_class = RoleMembershipSerializer queryset = RoleMembership.objects.select_related("role") filter_rewrite_rules = {"organization_id": "role__organization_id"} diff --git a/ee/api/rbac/test/test_access_control.py b/ee/api/rbac/test/test_access_control.py new file mode 100644 index 0000000000000..6a0b7a60519c7 --- /dev/null +++ b/ee/api/rbac/test/test_access_control.py @@ -0,0 +1,598 @@ +import json +from unittest.mock import MagicMock, patch +from rest_framework import status + +from ee.api.test.base import APILicensedTest +from ee.models.rbac.role import Role, RoleMembership +from posthog.constants import AvailableFeature +from posthog.models.dashboard import Dashboard +from posthog.models.feature_flag.feature_flag import FeatureFlag +from posthog.models.notebook.notebook import Notebook +from posthog.models.organization import OrganizationMembership +from posthog.models.team.team import Team +from posthog.utils import render_template + + +class BaseAccessControlTest(APILicensedTest): + def setUp(self): + super().setUp() + self.organization.available_features = [ + AvailableFeature.PROJECT_BASED_PERMISSIONING, + AvailableFeature.ROLE_BASED_ACCESS, + ] + self.organization.save() + + def _put_project_access_control(self, data=None): + payload = {"access_level": "admin"} + + if data: + payload.update(data) + + return self.client.put( + "/api/projects/@current/access_controls", + payload, + ) + + def _put_global_access_control(self, data=None): + payload = {"access_level": "editor"} + if data: + payload.update(data) + + return self.client.put( + "/api/projects/@current/global_access_controls", + payload, + ) + + def _org_membership(self, level: OrganizationMembership.Level = OrganizationMembership.Level.ADMIN): + self.organization_membership.level = level + self.organization_membership.save() + + +class TestAccessControlProjectLevelAPI(BaseAccessControlTest): + def test_project_change_rejected_if_not_org_admin(self): + self._org_membership(OrganizationMembership.Level.MEMBER) + res = self._put_project_access_control() + assert res.status_code == status.HTTP_403_FORBIDDEN, res.json() + + def test_project_change_accepted_if_org_admin(self): + self._org_membership(OrganizationMembership.Level.ADMIN) + res = self._put_project_access_control() + assert res.status_code == status.HTTP_200_OK, res.json() + + def test_project_change_accepted_if_org_owner(self): + self._org_membership(OrganizationMembership.Level.OWNER) + res = self._put_project_access_control() + assert res.status_code == status.HTTP_200_OK, res.json() + + def test_project_removed_with_null(self): + self._org_membership(OrganizationMembership.Level.OWNER) + res = self._put_project_access_control() + res = self._put_project_access_control({"access_level": None}) + assert res.status_code == status.HTTP_204_NO_CONTENT + + def test_project_change_if_in_access_control(self): + self._org_membership(OrganizationMembership.Level.ADMIN) + # Add ourselves to access + res = self._put_project_access_control( + {"organization_member": str(self.organization_membership.id), "access_level": "admin"} + ) + assert res.status_code == status.HTTP_200_OK, res.json() + + self._org_membership(OrganizationMembership.Level.MEMBER) + + # Now change ourselves to a member + res = self._put_project_access_control( + {"organization_member": str(self.organization_membership.id), "access_level": "member"} + ) + assert res.status_code == status.HTTP_200_OK, res.json() + assert res.json()["access_level"] == "member" + + # Now try and change our own membership and fail! + res = self._put_project_access_control( + {"organization_member": str(self.organization_membership.id), "access_level": "admin"} + ) + assert res.status_code == status.HTTP_403_FORBIDDEN + assert res.json()["detail"] == "Must be admin to modify project permissions." + + def test_project_change_rejected_if_not_in_organization(self): + self.organization_membership.delete() + res = self._put_project_access_control( + {"organization_member": str(self.organization_membership.id), "access_level": "admin"} + ) + assert res.status_code == status.HTTP_404_NOT_FOUND, res.json() + + def test_project_change_rejected_if_bad_access_level(self): + self._org_membership(OrganizationMembership.Level.ADMIN) + res = self._put_project_access_control({"access_level": "bad"}) + assert res.status_code == status.HTTP_400_BAD_REQUEST, res.json() + assert res.json()["detail"] == "Invalid access level. Must be one of: none, member, admin", res.json() + + +class TestAccessControlResourceLevelAPI(BaseAccessControlTest): + def setUp(self): + super().setUp() + + self.notebook = Notebook.objects.create( + team=self.team, created_by=self.user, short_id="0", title="first notebook" + ) + + self.other_user = self._create_user("other_user") + self.other_user_notebook = Notebook.objects.create( + team=self.team, created_by=self.other_user, short_id="1", title="first notebook" + ) + + def _get_access_controls(self): + return self.client.get(f"/api/projects/@current/notebooks/{self.notebook.short_id}/access_controls") + + def _put_access_control(self, data=None, notebook_id=None): + payload = { + "access_level": "editor", + } + + if data: + payload.update(data) + return self.client.put( + f"/api/projects/@current/notebooks/{notebook_id or self.notebook.short_id}/access_controls", + payload, + ) + + def test_get_access_controls(self): + self._org_membership(OrganizationMembership.Level.MEMBER) + res = self._get_access_controls() + assert res.status_code == status.HTTP_200_OK, res.json() + assert res.json() == { + "access_controls": [], + "available_access_levels": ["none", "viewer", "editor"], + "user_access_level": "editor", + "default_access_level": "editor", + "user_can_edit_access_levels": True, + } + + def test_change_rejected_if_not_org_admin(self): + self._org_membership(OrganizationMembership.Level.MEMBER) + res = self._put_access_control(notebook_id=self.other_user_notebook.short_id) + assert res.status_code == status.HTTP_403_FORBIDDEN, res.json() + + def test_change_accepted_if_org_admin(self): + self._org_membership(OrganizationMembership.Level.ADMIN) + res = self._put_access_control(notebook_id=self.other_user_notebook.short_id) + assert res.status_code == status.HTTP_200_OK, res.json() + + def test_change_accepted_if_creator_of_the_resource(self): + self._org_membership(OrganizationMembership.Level.MEMBER) + res = self._put_access_control(notebook_id=self.notebook.short_id) + assert res.status_code == status.HTTP_200_OK, res.json() + + +class TestGlobalAccessControlsPermissions(BaseAccessControlTest): + def setUp(self): + super().setUp() + + self.role = Role.objects.create(name="Engineers", organization=self.organization) + self.role_membership = RoleMembership.objects.create(user=self.user, role=self.role) + + def test_admin_can_always_access(self): + self._org_membership(OrganizationMembership.Level.ADMIN) + assert ( + self._put_global_access_control({"resource": "feature_flag", "access_level": "none"}).status_code + == status.HTTP_200_OK + ) + assert self.client.get("/api/projects/@current/feature_flags").status_code == status.HTTP_200_OK + + def test_forbidden_access_if_resource_wide_control_in_place(self): + self._org_membership(OrganizationMembership.Level.ADMIN) + assert ( + self._put_global_access_control({"resource": "feature_flag", "access_level": "none"}).status_code + == status.HTTP_200_OK + ) + self._org_membership(OrganizationMembership.Level.MEMBER) + + assert self.client.get("/api/projects/@current/feature_flags").status_code == status.HTTP_403_FORBIDDEN + assert self.client.post("/api/projects/@current/feature_flags").status_code == status.HTTP_403_FORBIDDEN + + def test_forbidden_write_access_if_resource_wide_control_in_place(self): + self._org_membership(OrganizationMembership.Level.ADMIN) + assert ( + self._put_global_access_control({"resource": "feature_flag", "access_level": "viewer"}).status_code + == status.HTTP_200_OK + ) + self._org_membership(OrganizationMembership.Level.MEMBER) + + assert self.client.get("/api/projects/@current/feature_flags").status_code == status.HTTP_200_OK + assert self.client.post("/api/projects/@current/feature_flags").status_code == status.HTTP_403_FORBIDDEN + + def test_access_granted_with_granted_role(self): + self._org_membership(OrganizationMembership.Level.ADMIN) + assert ( + self._put_global_access_control({"resource": "feature_flag", "access_level": "none"}).status_code + == status.HTTP_200_OK + ) + assert ( + self._put_global_access_control( + {"resource": "feature_flag", "access_level": "viewer", "role": self.role.id} + ).status_code + == status.HTTP_200_OK + ) + self._org_membership(OrganizationMembership.Level.MEMBER) + + assert self.client.get("/api/projects/@current/feature_flags").status_code == status.HTTP_200_OK + assert self.client.post("/api/projects/@current/feature_flags").status_code == status.HTTP_403_FORBIDDEN + + self.role_membership.delete() + assert self.client.get("/api/projects/@current/feature_flags").status_code == status.HTTP_403_FORBIDDEN + + +class TestAccessControlPermissions(BaseAccessControlTest): + """ + Test actual permissions being applied for a resource (notebooks as an example) + """ + + def setUp(self): + super().setUp() + self.other_user = self._create_user("other_user") + + self.other_user_notebook = Notebook.objects.create( + team=self.team, created_by=self.other_user, title="not my notebook" + ) + + self.notebook = Notebook.objects.create(team=self.team, created_by=self.user, title="my notebook") + + def _post_notebook(self): + return self.client.post("/api/projects/@current/notebooks/", {"title": "notebook"}) + + def _patch_notebook(self, id: str): + return self.client.patch(f"/api/projects/@current/notebooks/{id}", {"title": "new-title"}) + + def _get_notebook(self, id: str): + return self.client.get(f"/api/projects/@current/notebooks/{id}") + + def _put_notebook_access_control(self, notebook_id: str, data=None): + payload = { + "access_level": "editor", + } + + if data: + payload.update(data) + return self.client.put( + f"/api/projects/@current/notebooks/{notebook_id}/access_controls", + payload, + ) + + def test_default_allows_all_access(self): + self._org_membership(OrganizationMembership.Level.MEMBER) + assert self._get_notebook(self.other_user_notebook.short_id).status_code == status.HTTP_200_OK + assert self._patch_notebook(id=self.other_user_notebook.short_id).status_code == status.HTTP_200_OK + res = self._post_notebook() + assert res.status_code == status.HTTP_201_CREATED + assert self._patch_notebook(id=res.json()["short_id"]).status_code == status.HTTP_200_OK + + def test_rejects_all_access_without_project_access(self): + self._org_membership(OrganizationMembership.Level.ADMIN) + assert self._put_project_access_control({"access_level": "none"}).status_code == status.HTTP_200_OK + self._org_membership(OrganizationMembership.Level.MEMBER) + + assert self._get_notebook(self.other_user_notebook.short_id).status_code == status.HTTP_403_FORBIDDEN + assert self._patch_notebook(id=self.other_user_notebook.short_id).status_code == status.HTTP_403_FORBIDDEN + assert self._post_notebook().status_code == status.HTTP_403_FORBIDDEN + + def test_permits_access_with_member_control(self): + self._org_membership(OrganizationMembership.Level.ADMIN) + assert self._put_project_access_control({"access_level": "none"}).status_code == status.HTTP_200_OK + assert ( + self._put_project_access_control( + {"access_level": "member", "organization_member": str(self.organization_membership.id)} + ).status_code + == status.HTTP_200_OK + ) + self._org_membership(OrganizationMembership.Level.MEMBER) + + assert self._get_notebook(self.other_user_notebook.short_id).status_code == status.HTTP_200_OK + assert self._patch_notebook(id=self.other_user_notebook.short_id).status_code == status.HTTP_200_OK + assert self._post_notebook().status_code == status.HTTP_201_CREATED + + def test_rejects_edit_access_with_resource_control(self): + self._org_membership(OrganizationMembership.Level.ADMIN) + # Set other notebook to only allow view access by default + assert ( + self._put_notebook_access_control(self.other_user_notebook.short_id, {"access_level": "viewer"}).status_code + == status.HTTP_200_OK + ) + self._org_membership(OrganizationMembership.Level.MEMBER) + + assert self._get_notebook(self.other_user_notebook.short_id).status_code == status.HTTP_200_OK + assert self._patch_notebook(id=self.other_user_notebook.short_id).status_code == status.HTTP_403_FORBIDDEN + assert self._post_notebook().status_code == status.HTTP_201_CREATED + + def test_rejects_view_access_if_not_creator(self): + self._org_membership(OrganizationMembership.Level.ADMIN) + # Set other notebook to only allow view access by default + assert ( + self._put_notebook_access_control(self.other_user_notebook.short_id, {"access_level": "none"}).status_code + == status.HTTP_200_OK + ) + assert ( + self._put_notebook_access_control(self.notebook.short_id, {"access_level": "none"}).status_code + == status.HTTP_200_OK + ) + self._org_membership(OrganizationMembership.Level.MEMBER) + + # Access to other notebook is denied + assert self._get_notebook(self.other_user_notebook.short_id).status_code == status.HTTP_403_FORBIDDEN + assert self._patch_notebook(id=self.other_user_notebook.short_id).status_code == status.HTTP_403_FORBIDDEN + # As creator, access to my notebook is still permitted + assert self._get_notebook(self.notebook.short_id).status_code == status.HTTP_200_OK + assert self._patch_notebook(id=self.notebook.short_id).status_code == status.HTTP_200_OK + + def test_org_level_endpoints_work(self): + assert self.client.get("/api/organizations/@current/plugins").status_code == status.HTTP_200_OK + + +class TestAccessControlQueryCounts(BaseAccessControlTest): + def setUp(self): + super().setUp() + self.other_user = self._create_user("other_user") + + self.other_user_notebook = Notebook.objects.create( + team=self.team, created_by=self.other_user, title="not my notebook" + ) + + self.notebook = Notebook.objects.create(team=self.team, created_by=self.user, title="my notebook") + + # Baseline call to trigger caching of one off things like instance settings + self.client.get(f"/api/projects/@current/notebooks/{self.notebook.short_id}") + + def test_query_counts(self): + self._org_membership(OrganizationMembership.Level.MEMBER) + my_dashboard = Dashboard.objects.create(team=self.team, created_by=self.user) + other_user_dashboard = Dashboard.objects.create(team=self.team, created_by=self.other_user) + + # Baseline query (triggers any first time cache things) + self.client.get(f"/api/projects/@current/notebooks/{self.notebook.short_id}") + baseline = 11 + + # Access controls total 2 extra queries - 1 for org membership, 1 for the user roles, 1 for the preloaded access controls + with self.assertNumQueries(baseline + 3): + self.client.get(f"/api/projects/@current/dashboards/{my_dashboard.id}?no_items_field=true") + + # Accessing a different users dashboard doesn't +1 as the preload works using the pk + with self.assertNumQueries(baseline + 3): + self.client.get(f"/api/projects/@current/dashboards/{other_user_dashboard.id}?no_items_field=true") + + baseline = 6 + # Getting my own notebook is the same as a dashboard - 2 extra queries + with self.assertNumQueries(baseline + 3): + self.client.get(f"/api/projects/@current/notebooks/{self.notebook.short_id}") + + # Except when accessing a different notebook where we _also_ need to check as we are not the creator and the pk is not the same (short_id) + with self.assertNumQueries(baseline + 4): + self.client.get(f"/api/projects/@current/notebooks/{self.other_user_notebook.short_id}") + + baseline = 4 + # Project access doesn't double query the object + with self.assertNumQueries(baseline + 3): + # We call this endpoint as we don't want to include all the extra queries that rendering the project uses + self.client.get("/api/projects/@current/is_generating_demo_data") + + # When accessing the list of notebooks we have extra queries due to checking for role based access and filtering out items + baseline = 7 + with self.assertNumQueries(baseline + 3): # org, roles, preloaded access controls + self.client.get("/api/projects/@current/notebooks/") + + def test_query_counts_with_preload_optimization(self): + self._org_membership(OrganizationMembership.Level.MEMBER) + my_dashboard = Dashboard.objects.create(team=self.team, created_by=self.user) + other_user_dashboard = Dashboard.objects.create(team=self.team, created_by=self.other_user) + + # Baseline query (triggers any first time cache things) + self.client.get(f"/api/projects/@current/notebooks/{self.notebook.short_id}") + baseline = 11 + + # Access controls total 2 extra queries - 1 for org membership, 1 for the user roles, 1 for the preloaded access controls + with self.assertNumQueries(baseline + 3): + self.client.get(f"/api/projects/@current/dashboards/{my_dashboard.id}?no_items_field=true") + + # Accessing a different users dashboard doesn't +1 as the preload works using the pk + with self.assertNumQueries(baseline + 3): + self.client.get(f"/api/projects/@current/dashboards/{other_user_dashboard.id}?no_items_field=true") + + def test_query_counts_only_adds_1_for_non_pk_resources(self): + self._org_membership(OrganizationMembership.Level.MEMBER) + # Baseline query (triggers any first time cache things) + self.client.get(f"/api/projects/@current/notebooks/{self.notebook.short_id}") + baseline = 11 + + baseline = 6 + # Getting my own notebook is the same as a dashboard - 2 extra queries + with self.assertNumQueries(baseline + 3): + self.client.get(f"/api/projects/@current/notebooks/{self.notebook.short_id}") + + # Except when accessing a different notebook where we _also_ need to check as we are not the creator and the pk is not the same (short_id) + with self.assertNumQueries(baseline + 4): + self.client.get(f"/api/projects/@current/notebooks/{self.other_user_notebook.short_id}") + + def test_query_counts_stable_for_project_access(self): + self._org_membership(OrganizationMembership.Level.MEMBER) + baseline = 4 + # Project access doesn't double query the object + with self.assertNumQueries(baseline + 3): + # We call this endpoint as we don't want to include all the extra queries that rendering the project uses + self.client.get("/api/projects/@current/is_generating_demo_data") + + # When accessing the list of notebooks we have extra queries due to checking for role based access and filtering out items + baseline = 7 + with self.assertNumQueries(baseline + 3): # org, roles, preloaded access controls + self.client.get("/api/projects/@current/notebooks/") + + def test_query_counts_stable_when_listing_resources(self): + # When accessing the list of notebooks we have extra queries due to checking for role based access and filtering out items + baseline = 7 + with self.assertNumQueries(baseline + 3): # org, roles, preloaded access controls + self.client.get("/api/projects/@current/notebooks/") + + def test_query_counts_stable_when_listing_resources_including_access_control_info(self): + for i in range(10): + FeatureFlag.objects.create(team=self.team, created_by=self.other_user, key=f"flag-{i}") + + baseline = 42 # This is a lot! There is currently an n+1 issue with the legacy access control system + with self.assertNumQueries(baseline + 4): # org, roles, preloaded permissions acs, preloaded acs for the list + self.client.get("/api/projects/@current/feature_flags/") + + for i in range(10): + FeatureFlag.objects.create(team=self.team, created_by=self.other_user, key=f"flag-{10 + i}") + + baseline = baseline + (10 * 3) # The existing access control adds 3 queries per item :( + with self.assertNumQueries(baseline + 4): # org, roles, preloaded permissions acs, preloaded acs for the list + self.client.get("/api/projects/@current/feature_flags/") + + +class TestAccessControlFiltering(BaseAccessControlTest): + def setUp(self): + super().setUp() + self.other_user = self._create_user("other_user") + + self.other_user_notebook = Notebook.objects.create( + team=self.team, created_by=self.other_user, title="not my notebook" + ) + + self.notebook = Notebook.objects.create(team=self.team, created_by=self.user, title="my notebook") + + def _put_notebook_access_control(self, notebook_id: str, data=None): + payload = { + "access_level": "editor", + } + + if data: + payload.update(data) + return self.client.put( + f"/api/projects/@current/notebooks/{notebook_id}/access_controls", + payload, + ) + + def _get_notebooks(self): + return self.client.get("/api/projects/@current/notebooks/") + + def test_default_allows_all_access(self): + self._org_membership(OrganizationMembership.Level.MEMBER) + assert len(self._get_notebooks().json()["results"]) == 2 + + def test_does_not_list_notebooks_without_access(self): + self._org_membership(OrganizationMembership.Level.ADMIN) + assert ( + self._put_notebook_access_control(self.other_user_notebook.short_id, {"access_level": "none"}).status_code + == status.HTTP_200_OK + ) + assert ( + self._put_notebook_access_control(self.notebook.short_id, {"access_level": "none"}).status_code + == status.HTTP_200_OK + ) + self._org_membership(OrganizationMembership.Level.MEMBER) + + res = self._get_notebooks() + assert len(res.json()["results"]) == 1 + assert res.json()["results"][0]["id"] == str(self.notebook.id) + + def test_list_notebooks_with_explicit_access(self): + self._org_membership(OrganizationMembership.Level.ADMIN) + assert ( + self._put_notebook_access_control(self.other_user_notebook.short_id, {"access_level": "none"}).status_code + == status.HTTP_200_OK + ) + assert ( + self._put_notebook_access_control( + self.other_user_notebook.short_id, + {"organization_member": str(self.organization_membership.id), "access_level": "viewer"}, + ).status_code + == status.HTTP_200_OK + ) + self._org_membership(OrganizationMembership.Level.MEMBER) + + res = self._get_notebooks() + assert len(res.json()["results"]) == 2 + + def test_search_results_exclude_restricted_objects(self): + res = self.client.get("/api/projects/@current/search?q=my notebook") + assert len(res.json()["results"]) == 2 + + self._org_membership(OrganizationMembership.Level.ADMIN) + assert ( + self._put_notebook_access_control(self.other_user_notebook.short_id, {"access_level": "none"}).status_code + == status.HTTP_200_OK + ) + + self._org_membership(OrganizationMembership.Level.MEMBER) + + res = self.client.get("/api/projects/@current/search?q=my notebook") + assert len(res.json()["results"]) == 1 + + +class TestAccessControlProjectFiltering(BaseAccessControlTest): + """ + Projects are listed in multiple places and ways so we need to test all of them here + """ + + def setUp(self): + super().setUp() + self.other_team = Team.objects.create(organization=self.organization, name="other team") + self.other_team_2 = Team.objects.create(organization=self.organization, name="other team 2") + + def _put_project_access_control_as_admin(self, team_id: int, data=None): + self._org_membership(OrganizationMembership.Level.ADMIN) + payload = { + "access_level": "editor", + } + + if data: + payload.update(data) + res = self.client.put( + f"/api/projects/{team_id}/access_controls", + payload, + ) + + self._org_membership(OrganizationMembership.Level.MEMBER) + + assert res.status_code == status.HTTP_200_OK, res.json() + return res + + def _get_posthog_app_context(self): + mock_template = MagicMock() + with patch("posthog.utils.get_template", return_value=mock_template): + mock_request = MagicMock() + mock_request.user = self.user + mock_request.GET = {} + render_template("index.html", request=mock_request, context={}) + + # Get the context passed to the template + return json.loads(mock_template.render.call_args[0][0]["posthog_app_context"]) + + def test_default_lists_all_projects(self): + assert len(self.client.get("/api/projects").json()["results"]) == 3 + me_response = self.client.get("/api/users/@me").json() + assert len(me_response["organization"]["teams"]) == 3 + + def test_does_not_list_projects_without_access(self): + self._put_project_access_control_as_admin(self.other_team.id, {"access_level": "none"}) + assert len(self.client.get("/api/projects").json()["results"]) == 2 + me_response = self.client.get("/api/users/@me").json() + assert len(me_response["organization"]["teams"]) == 2 + + def test_always_lists_all_projects_if_org_admin(self): + self._put_project_access_control_as_admin(self.other_team.id, {"access_level": "none"}) + self._org_membership(OrganizationMembership.Level.ADMIN) + assert len(self.client.get("/api/projects").json()["results"]) == 3 + me_response = self.client.get("/api/users/@me").json() + assert len(me_response["organization"]["teams"]) == 3 + + def test_template_render_filters_teams(self): + app_context = self._get_posthog_app_context() + assert len(app_context["current_user"]["organization"]["teams"]) == 3 + assert app_context["current_team"]["id"] == self.team.id + assert app_context["current_team"]["user_access_level"] == "admin" + + self._put_project_access_control_as_admin(self.team.id, {"access_level": "none"}) + app_context = self._get_posthog_app_context() + assert len(app_context["current_user"]["organization"]["teams"]) == 2 + assert app_context["current_team"]["id"] == self.team.id + assert app_context["current_team"]["user_access_level"] == "none" + + +# TODO: Add tests to check that a dashboard can't be edited if the user doesn't have access diff --git a/ee/api/test/test_feature_flag.py b/ee/api/test/test_feature_flag.py index e3dd5849d607c..0bc7292f7a875 100644 --- a/ee/api/test/test_feature_flag.py +++ b/ee/api/test/test_feature_flag.py @@ -1,6 +1,6 @@ from ee.api.test.base import APILicensedTest -from ee.models.organization_resource_access import OrganizationResourceAccess -from ee.models.role import Role, RoleMembership +from ee.models.rbac.organization_resource_access import OrganizationResourceAccess +from ee.models.rbac.role import Role, RoleMembership from posthog.models.feature_flag import FeatureFlag from posthog.models.organization import OrganizationMembership diff --git a/ee/api/test/test_feature_flag_role_access.py b/ee/api/test/test_feature_flag_role_access.py index 3cd4e947d90c9..d73c1c7384493 100644 --- a/ee/api/test/test_feature_flag_role_access.py +++ b/ee/api/test/test_feature_flag_role_access.py @@ -2,8 +2,8 @@ from ee.api.test.base import APILicensedTest from ee.models.feature_flag_role_access import FeatureFlagRoleAccess -from ee.models.organization_resource_access import OrganizationResourceAccess -from ee.models.role import Role +from ee.models.rbac.organization_resource_access import OrganizationResourceAccess +from ee.models.rbac.role import Role from posthog.models.feature_flag import FeatureFlag from posthog.models.organization import OrganizationMembership from posthog.models.user import User diff --git a/ee/api/test/test_organization_resource_access.py b/ee/api/test/test_organization_resource_access.py index 9123214a092db..98206fe519f44 100644 --- a/ee/api/test/test_organization_resource_access.py +++ b/ee/api/test/test_organization_resource_access.py @@ -2,7 +2,7 @@ from rest_framework import status from ee.api.test.base import APILicensedTest -from ee.models.organization_resource_access import OrganizationResourceAccess +from ee.models.rbac.organization_resource_access import OrganizationResourceAccess from posthog.models.organization import Organization, OrganizationMembership from posthog.test.base import QueryMatchingTest, snapshot_postgres_queries, FuzzyInt diff --git a/ee/api/test/test_role.py b/ee/api/test/test_role.py index 1a3068ff4cf4f..96503162d5fe9 100644 --- a/ee/api/test/test_role.py +++ b/ee/api/test/test_role.py @@ -2,8 +2,8 @@ from rest_framework import status from ee.api.test.base import APILicensedTest -from ee.models.organization_resource_access import OrganizationResourceAccess -from ee.models.role import Role +from ee.models.rbac.organization_resource_access import OrganizationResourceAccess +from ee.models.rbac.role import Role from posthog.models.organization import Organization, OrganizationMembership diff --git a/ee/api/test/test_role_membership.py b/ee/api/test/test_role_membership.py index f89796d9b7c4f..c3e67cf0514d2 100644 --- a/ee/api/test/test_role_membership.py +++ b/ee/api/test/test_role_membership.py @@ -1,7 +1,7 @@ from rest_framework import status from ee.api.test.base import APILicensedTest -from ee.models.role import Role, RoleMembership +from ee.models.rbac.role import Role, RoleMembership from posthog.models.organization import Organization, OrganizationMembership from posthog.models.user import User diff --git a/ee/clickhouse/views/test/test_clickhouse_experiments.py b/ee/clickhouse/views/test/test_clickhouse_experiments.py index fdd0c05656c7c..a2194fbcb0ecf 100644 --- a/ee/clickhouse/views/test/test_clickhouse_experiments.py +++ b/ee/clickhouse/views/test/test_clickhouse_experiments.py @@ -55,7 +55,7 @@ def test_getting_experiments_is_not_nplus1(self) -> None: format="json", ).json() - with self.assertNumQueries(FuzzyInt(8, 9)): + with self.assertNumQueries(FuzzyInt(10, 11)): response = self.client.get(f"/api/projects/{self.team.id}/experiments") self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -1011,7 +1011,7 @@ def test_used_in_experiment_is_populated_correctly_for_feature_flag_list(self) - ).json() # TODO: Make sure permission bool doesn't cause n + 1 - with self.assertNumQueries(12): + with self.assertNumQueries(16): response = self.client.get(f"/api/projects/{self.team.id}/feature_flags") self.assertEqual(response.status_code, status.HTTP_200_OK) result = response.json() diff --git a/ee/migrations/0017_accesscontrol.py b/ee/migrations/0017_accesscontrol.py new file mode 100644 index 0000000000000..e69281b7dcca4 --- /dev/null +++ b/ee/migrations/0017_accesscontrol.py @@ -0,0 +1,75 @@ +# Generated by Django 4.1.13 on 2024-03-19 14:02 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import posthog.models.utils + + +class Migration(migrations.Migration): + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ("posthog", "0397_projects_backfill"), + ("ee", "0016_rolemembership_organization_member"), + ] + + operations = [ + migrations.CreateModel( + name="AccessControl", + fields=[ + ( + "id", + models.UUIDField( + default=posthog.models.utils.UUIDT, editable=False, primary_key=True, serialize=False + ), + ), + ("resource", models.CharField(max_length=32)), + ("access_level", models.CharField(max_length=32)), + ("resource_id", models.CharField(max_length=36, null=True)), + ("created_at", models.DateTimeField(auto_now_add=True)), + ("updated_at", models.DateTimeField(auto_now=True)), + ( + "created_by", + models.ForeignKey( + null=True, on_delete=django.db.models.deletion.SET_NULL, to=settings.AUTH_USER_MODEL + ), + ), + ( + "organization_member", + models.ForeignKey( + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="access_controls", + related_query_name="access_controls", + to="posthog.organizationmembership", + ), + ), + ( + "role", + models.ForeignKey( + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="access_controls", + related_query_name="access_controls", + to="ee.role", + ), + ), + ( + "team", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="access_controls", + related_query_name="access_controls", + to="posthog.team", + ), + ), + ], + ), + migrations.AddConstraint( + model_name="accesscontrol", + constraint=models.UniqueConstraint( + fields=("resource", "resource_id", "team", "organization_member", "role"), + name="unique resource per target", + ), + ), + ] diff --git a/ee/models/__init__.py b/ee/models/__init__.py index fd87f76bd54eb..137f965a5c06c 100644 --- a/ee/models/__init__.py +++ b/ee/models/__init__.py @@ -5,9 +5,11 @@ from .hook import Hook from .license import License from .property_definition import EnterprisePropertyDefinition -from .role import Role, RoleMembership +from .rbac.role import Role, RoleMembership +from .rbac.access_control import AccessControl __all__ = [ + "AccessControl", "EnterpriseEventDefinition", "ExplicitTeamMembership", "DashboardPrivilege", diff --git a/ee/models/feature_flag_role_access.py b/ee/models/feature_flag_role_access.py index 867f2d562b944..c9649fc7e5e2f 100644 --- a/ee/models/feature_flag_role_access.py +++ b/ee/models/feature_flag_role_access.py @@ -1,6 +1,9 @@ from django.db import models +# NOTE: This will be deprecated in favour of the AccessControl model + + class FeatureFlagRoleAccess(models.Model): feature_flag = models.ForeignKey( "posthog.FeatureFlag", diff --git a/ee/models/rbac/access_control.py b/ee/models/rbac/access_control.py new file mode 100644 index 0000000000000..bae4f582c1f7f --- /dev/null +++ b/ee/models/rbac/access_control.py @@ -0,0 +1,51 @@ +from django.db import models + +from posthog.models.utils import UUIDModel + + +class AccessControl(UUIDModel): + class Meta: + constraints = [ + models.UniqueConstraint( + fields=["resource", "resource_id", "team", "organization_member", "role"], + name="unique resource per target", + ) + ] + + team: models.ForeignKey = models.ForeignKey( + "posthog.Team", + on_delete=models.CASCADE, + related_name="access_controls", + related_query_name="access_controls", + ) + + # Configuration of what we are accessing + resource: models.CharField = models.CharField(max_length=32) + access_level: models.CharField = models.CharField(max_length=32) + resource_id: models.CharField = models.CharField(max_length=36, null=True) + + # Optional scope it to a specific member + organization_member: models.ForeignKey = models.ForeignKey( + "posthog.OrganizationMembership", + on_delete=models.CASCADE, + related_name="access_controls", + related_query_name="access_controls", + null=True, + ) + + # Optional scope it to a specific role + role: models.ForeignKey = models.ForeignKey( + "Role", + on_delete=models.CASCADE, + related_name="access_controls", + related_query_name="access_controls", + null=True, + ) + + created_by: models.ForeignKey = models.ForeignKey( + "posthog.User", + on_delete=models.SET_NULL, + null=True, + ) + created_at: models.DateTimeField = models.DateTimeField(auto_now_add=True) + updated_at: models.DateTimeField = models.DateTimeField(auto_now=True) diff --git a/ee/models/organization_resource_access.py b/ee/models/rbac/organization_resource_access.py similarity index 95% rename from ee/models/organization_resource_access.py rename to ee/models/rbac/organization_resource_access.py index 924b3e9db2855..de4c86d95a8bc 100644 --- a/ee/models/organization_resource_access.py +++ b/ee/models/rbac/organization_resource_access.py @@ -2,6 +2,8 @@ from posthog.models.organization import Organization +# NOTE: This will be deprecated in favour of the AccessControl model + class OrganizationResourceAccess(models.Model): class AccessLevel(models.IntegerChoices): diff --git a/ee/models/role.py b/ee/models/rbac/role.py similarity index 90% rename from ee/models/role.py rename to ee/models/rbac/role.py index f37170818dbc3..be352a35dbfe0 100644 --- a/ee/models/role.py +++ b/ee/models/rbac/role.py @@ -1,21 +1,21 @@ from django.db import models -from ee.models.organization_resource_access import OrganizationResourceAccess +from ee.models.rbac.organization_resource_access import OrganizationResourceAccess from posthog.models.utils import UUIDModel class Role(UUIDModel): - name = models.CharField(max_length=200) + class Meta: + constraints = [models.UniqueConstraint(fields=["organization", "name"], name="unique_role_name")] + + name: models.CharField = models.CharField(max_length=200) organization = models.ForeignKey( "posthog.Organization", on_delete=models.CASCADE, related_name="roles", related_query_name="role", ) - feature_flags_access_level = models.PositiveSmallIntegerField( - default=OrganizationResourceAccess.AccessLevel.CAN_ALWAYS_EDIT, - choices=OrganizationResourceAccess.AccessLevel.choices, - ) + created_at = models.DateTimeField(auto_now_add=True) created_by = models.ForeignKey( "posthog.User", @@ -25,11 +25,17 @@ class Role(UUIDModel): null=True, ) - class Meta: - constraints = [models.UniqueConstraint(fields=["organization", "name"], name="unique_role_name")] + # TODO: Deprecate this field + feature_flags_access_level = models.PositiveSmallIntegerField( + default=OrganizationResourceAccess.AccessLevel.CAN_ALWAYS_EDIT, + choices=OrganizationResourceAccess.AccessLevel.choices, + ) class RoleMembership(UUIDModel): + class Meta: + constraints = [models.UniqueConstraint(fields=["role", "user"], name="unique_user_and_role")] + role = models.ForeignKey( "Role", on_delete=models.CASCADE, @@ -53,6 +59,3 @@ class RoleMembership(UUIDModel): ) joined_at = models.DateTimeField(auto_now_add=True) updated_at = models.DateTimeField(auto_now=True) - - class Meta: - constraints = [models.UniqueConstraint(fields=["role", "user"], name="unique_user_and_role")] diff --git a/ee/urls.py b/ee/urls.py index f0cf168acffb0..e9e9e804d8bc8 100644 --- a/ee/urls.py +++ b/ee/urls.py @@ -6,6 +6,7 @@ from django.urls.conf import path from ee.api import integration +from .api.rbac import organization_resource_access, role from .api import ( authentication, @@ -15,8 +16,6 @@ feature_flag_role_access, hooks, license, - organization_resource_access, - role, sentry_stats, subscription, ) @@ -49,6 +48,8 @@ def extend_api_router() -> None: "organization_role_memberships", ["organization_id", "role_id"], ) + + # ROUTES TO BE DEPRECATED project_feature_flags_router.register( r"role_access", feature_flag_role_access.FeatureFlagRoleAccessViewSet, diff --git a/frontend/__snapshots__/exporter-exporter--trends-line-insight-detailed--dark.png b/frontend/__snapshots__/exporter-exporter--trends-line-insight-detailed--dark.png index e3e93b3dd9678..b54d4facb0bfe 100644 Binary files a/frontend/__snapshots__/exporter-exporter--trends-line-insight-detailed--dark.png and b/frontend/__snapshots__/exporter-exporter--trends-line-insight-detailed--dark.png differ diff --git a/frontend/__snapshots__/exporter-exporter--trends-line-insight-detailed--light.png b/frontend/__snapshots__/exporter-exporter--trends-line-insight-detailed--light.png index d4377362d9270..332ea24ce3084 100644 Binary files a/frontend/__snapshots__/exporter-exporter--trends-line-insight-detailed--light.png and b/frontend/__snapshots__/exporter-exporter--trends-line-insight-detailed--light.png differ diff --git a/frontend/__snapshots__/scenes-app-sidepanels--side-panel-docs--dark.png b/frontend/__snapshots__/scenes-app-sidepanels--side-panel-docs--dark.png index 4fe845177fd6b..29fe5a51e80b4 100644 Binary files a/frontend/__snapshots__/scenes-app-sidepanels--side-panel-docs--dark.png and b/frontend/__snapshots__/scenes-app-sidepanels--side-panel-docs--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-sidepanels--side-panel-docs--light.png b/frontend/__snapshots__/scenes-app-sidepanels--side-panel-docs--light.png index d4cf49f5c030a..71f9dac33c3cd 100644 Binary files a/frontend/__snapshots__/scenes-app-sidepanels--side-panel-docs--light.png and b/frontend/__snapshots__/scenes-app-sidepanels--side-panel-docs--light.png differ diff --git a/frontend/__snapshots__/scenes-other-org-member-invites--current-user-is-admin--dark.png b/frontend/__snapshots__/scenes-other-org-member-invites--current-user-is-admin--dark.png index 2108bf08e480f..486f9f55b44a8 100644 Binary files a/frontend/__snapshots__/scenes-other-org-member-invites--current-user-is-admin--dark.png and b/frontend/__snapshots__/scenes-other-org-member-invites--current-user-is-admin--dark.png differ diff --git a/frontend/__snapshots__/scenes-other-org-member-invites--current-user-is-admin--light.png b/frontend/__snapshots__/scenes-other-org-member-invites--current-user-is-admin--light.png index e797cc6467cfc..7c07b7749c7f0 100644 Binary files a/frontend/__snapshots__/scenes-other-org-member-invites--current-user-is-admin--light.png and b/frontend/__snapshots__/scenes-other-org-member-invites--current-user-is-admin--light.png differ diff --git a/frontend/__snapshots__/scenes-other-org-member-invites--current-user-is-member--dark.png b/frontend/__snapshots__/scenes-other-org-member-invites--current-user-is-member--dark.png index 644dc1a22f0eb..8471e887c9326 100644 Binary files a/frontend/__snapshots__/scenes-other-org-member-invites--current-user-is-member--dark.png and b/frontend/__snapshots__/scenes-other-org-member-invites--current-user-is-member--dark.png differ diff --git a/frontend/__snapshots__/scenes-other-org-member-invites--current-user-is-member--light.png b/frontend/__snapshots__/scenes-other-org-member-invites--current-user-is-member--light.png index 750ff00450e15..b8714560d767e 100644 Binary files a/frontend/__snapshots__/scenes-other-org-member-invites--current-user-is-member--light.png and b/frontend/__snapshots__/scenes-other-org-member-invites--current-user-is-member--light.png differ diff --git a/frontend/__snapshots__/scenes-other-org-member-invites--current-user-is-owner--dark.png b/frontend/__snapshots__/scenes-other-org-member-invites--current-user-is-owner--dark.png index 2108bf08e480f..486f9f55b44a8 100644 Binary files a/frontend/__snapshots__/scenes-other-org-member-invites--current-user-is-owner--dark.png and b/frontend/__snapshots__/scenes-other-org-member-invites--current-user-is-owner--dark.png differ diff --git a/frontend/__snapshots__/scenes-other-org-member-invites--current-user-is-owner--light.png b/frontend/__snapshots__/scenes-other-org-member-invites--current-user-is-owner--light.png index e797cc6467cfc..7c07b7749c7f0 100644 Binary files a/frontend/__snapshots__/scenes-other-org-member-invites--current-user-is-owner--light.png and b/frontend/__snapshots__/scenes-other-org-member-invites--current-user-is-owner--light.png differ diff --git a/frontend/__snapshots__/scenes-other-settings--settings-organization--dark.png b/frontend/__snapshots__/scenes-other-settings--settings-organization--dark.png index c063bc66591f6..c98c9a6176da9 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-organization--dark.png and b/frontend/__snapshots__/scenes-other-settings--settings-organization--dark.png differ diff --git a/frontend/__snapshots__/scenes-other-settings--settings-organization--light.png b/frontend/__snapshots__/scenes-other-settings--settings-organization--light.png index 53d8c00a4492e..c88e0fb2a8112 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-organization--light.png and b/frontend/__snapshots__/scenes-other-settings--settings-organization--light.png differ diff --git a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-all-options--dark.png b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-all-options--dark.png index 7fbbd90b966bd..d00f4eb91e77c 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-all-options--dark.png and b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-all-options--dark.png differ diff --git a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-password-only--dark.png b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-password-only--dark.png index 2968881dd0f6d..d557025db0072 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-password-only--dark.png and b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-password-only--dark.png differ diff --git a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-password-only--light.png b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-password-only--light.png index aaeccae01012e..fd7b64b3cd88c 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-password-only--light.png and b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-password-only--light.png differ diff --git a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-github--dark.png b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-github--dark.png index b19c8bb0c1857..22d52363fccfe 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-github--dark.png and b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-github--dark.png differ diff --git a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-github--light.png b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-github--light.png index 2f248c1178b48..5db6a10ea7970 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-github--light.png and b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-github--light.png differ diff --git a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-google--dark.png b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-google--dark.png index 384888b49637f..2e63dc78b81bc 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-google--dark.png and b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-google--dark.png differ diff --git a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-google--light.png b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-google--light.png index fdfe3c5ff43cc..ea41527404ed8 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-google--light.png and b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-google--light.png differ diff --git a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-saml--dark.png b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-saml--dark.png index fa0e26b7b40d3..4d5085df70deb 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-saml--dark.png and b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-saml--dark.png differ diff --git a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-saml--light.png b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-saml--light.png index 72d7e2c71646b..39912ade9b954 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-saml--light.png and b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-saml--light.png differ diff --git a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-only--dark.png b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-only--dark.png index 94cf733f6bc9d..076823f9834d8 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-only--dark.png and b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-only--dark.png differ diff --git a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-only--light.png b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-only--light.png index c22c1112c9c7e..ae9dfc8d7b70f 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-only--light.png and b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-only--light.png differ diff --git a/frontend/src/layout/ErrorProjectUnavailable.tsx b/frontend/src/layout/ErrorProjectUnavailable.tsx index 8aa6719e4346b..8c02efae48972 100644 --- a/frontend/src/layout/ErrorProjectUnavailable.tsx +++ b/frontend/src/layout/ErrorProjectUnavailable.tsx @@ -2,6 +2,7 @@ import { Link } from '@posthog/lemon-ui' import { useValues } from 'kea' import { PageHeader } from 'lib/components/PageHeader' import { useEffect, useState } from 'react' +import { teamLogic } from 'scenes/teamLogic' import { urls } from 'scenes/urls' import { userLogic } from 'scenes/userLogic' @@ -10,6 +11,7 @@ import { organizationLogic } from '../scenes/organizationLogic' export function ErrorProjectUnavailable(): JSX.Element { const { projectCreationForbiddenReason } = useValues(organizationLogic) const { user } = useValues(userLogic) + const { currentTeam } = useValues(teamLogic) const [options, setOptions] = useState([]) useEffect(() => { @@ -42,7 +44,8 @@ export function ErrorProjectUnavailable(): JSX.Element { return (
- {user?.team && !user.organization?.teams.some((team) => team.id === user?.team?.id) ? ( + {(user?.team && !user.organization?.teams.some((team) => team.id === user?.team?.id || user.team)) || + currentTeam?.user_access_level === 'none' ? ( <>

Project access has been removed

diff --git a/frontend/src/layout/navigation-3000/sidepanel/SidePanel.tsx b/frontend/src/layout/navigation-3000/sidepanel/SidePanel.tsx index 972ca1515de48..257092539e987 100644 --- a/frontend/src/layout/navigation-3000/sidepanel/SidePanel.tsx +++ b/frontend/src/layout/navigation-3000/sidepanel/SidePanel.tsx @@ -1,6 +1,15 @@ import './SidePanel.scss' -import { IconEllipsis, IconFeatures, IconFlag, IconGear, IconInfo, IconNotebook, IconSupport } from '@posthog/icons' +import { + IconEllipsis, + IconFeatures, + IconFlag, + IconGear, + IconInfo, + IconLock, + IconNotebook, + IconSupport, +} from '@posthog/icons' import { LemonButton, LemonMenu, LemonMenuItems, LemonModal } from '@posthog/lemon-ui' import clsx from 'clsx' import { useActions, useValues } from 'kea' @@ -16,6 +25,7 @@ import { import { themeLogic } from '~/layout/navigation-3000/themeLogic' import { SidePanelTab } from '~/types' +import { SidePanelAccessControl } from './panels/access_control/SidePanelAccessControl' import { SidePanelActivation, SidePanelActivationIcon } from './panels/activation/SidePanelActivation' import { SidePanelActivity, SidePanelActivityIcon } from './panels/activity/SidePanelActivity' import { SidePanelDiscussion, SidePanelDiscussionIcon } from './panels/discussion/SidePanelDiscussion' @@ -88,6 +98,11 @@ export const SIDE_PANEL_TABS: Record< Content: SidePanelStatus, noModalSupport: true, }, + [SidePanelTab.AccessControl]: { + label: 'Access control', + Icon: IconLock, + Content: SidePanelAccessControl, + }, [SidePanelTab.ExperimentFeatureFlag]: { label: 'Release conditions', Icon: IconFlag, diff --git a/frontend/src/layout/navigation-3000/sidepanel/panels/access_control/AccessControlObject.tsx b/frontend/src/layout/navigation-3000/sidepanel/panels/access_control/AccessControlObject.tsx new file mode 100644 index 0000000000000..8dcff45165587 --- /dev/null +++ b/frontend/src/layout/navigation-3000/sidepanel/panels/access_control/AccessControlObject.tsx @@ -0,0 +1,438 @@ +import { IconX } from '@posthog/icons' +import { + LemonBanner, + LemonButton, + LemonDialog, + LemonInputSelect, + LemonSelect, + LemonSelectProps, + LemonTable, +} from '@posthog/lemon-ui' +import { BindLogic, useActions, useAsyncActions, useValues } from 'kea' +import { PayGateMini } from 'lib/components/PayGateMini/PayGateMini' +import { upgradeModalLogic } from 'lib/components/UpgradeModal/upgradeModalLogic' +import { UserSelectItem } from 'lib/components/UserSelectItem' +import { LemonTableColumns } from 'lib/lemon-ui/LemonTable' +import { LemonTableLink } from 'lib/lemon-ui/LemonTable/LemonTableLink' +import { ProfileBubbles, ProfilePicture } from 'lib/lemon-ui/ProfilePicture' +import { capitalizeFirstLetter } from 'lib/utils' +import { useEffect, useState } from 'react' +import { urls } from 'scenes/urls' +import { userLogic } from 'scenes/userLogic' + +import { + AccessControlType, + AccessControlTypeMember, + AccessControlTypeRole, + AvailableFeature, + OrganizationMemberType, +} from '~/types' + +import { accessControlLogic, AccessControlLogicProps } from './accessControlLogic' + +export function AccessControlObject(props: AccessControlLogicProps): JSX.Element | null { + const { canEditAccessControls, humanReadableResource } = useValues(accessControlLogic(props)) + + const suffix = `this ${humanReadableResource}` + + return ( + +

+ {canEditAccessControls === false ? ( + + You don't have permission to edit access controls for {suffix}. +
+ You must be the creator of it, a Project Admin, or an Organization Admin. +
+ ) : null} +

Default access to {suffix}

+ + +

Members with explicit access to {suffix}

+ + +

Roles with explicit access to {suffix}

+ + + +
+ + ) +} + +function AccessControlObjectDefaults(): JSX.Element | null { + const { accessControlDefault, accessControlDefaultOptions, accessControlsLoading, canEditAccessControls } = + useValues(accessControlLogic) + const { updateAccessControlDefault } = useActions(accessControlLogic) + const { guardAvailableFeature } = useValues(upgradeModalLogic) + + return ( + { + guardAvailableFeature(AvailableFeature.PROJECT_BASED_PERMISSIONING, () => { + updateAccessControlDefault(newValue) + }) + }} + disabledReason={ + accessControlsLoading ? 'Loading…' : !canEditAccessControls ? 'You cannot edit this' : undefined + } + dropdownMatchSelectWidth={false} + options={accessControlDefaultOptions} + /> + ) +} + +function AccessControlObjectUsers(): JSX.Element | null { + const { user } = useValues(userLogic) + const { membersById, addableMembers, accessControlMembers, accessControlsLoading, availableLevels } = + useValues(accessControlLogic) + const { updateAccessControlMembers } = useAsyncActions(accessControlLogic) + const { guardAvailableFeature } = useValues(upgradeModalLogic) + + if (!user) { + return null + } + + const member = (ac: AccessControlTypeMember): OrganizationMemberType => { + return membersById[ac.organization_member] + } + + // TODO: WHAT A MESS - Fix this to do the index mapping beforehand... + const columns: LemonTableColumns = [ + { + key: 'user_profile_picture', + render: function ProfilePictureRender(_, ac) { + return + }, + width: 32, + }, + { + title: 'Name', + key: 'user_first_name', + render: (_, ac) => ( + + {member(ac)?.user.uuid == user.uuid + ? `${member(ac)?.user.first_name} (you)` + : member(ac)?.user.first_name} + + ), + sorter: (a, b) => member(a)?.user.first_name.localeCompare(member(b)?.user.first_name), + }, + { + title: 'Email', + key: 'user_email', + render: (_, ac) => member(ac)?.user.email, + sorter: (a, b) => member(a)?.user.email.localeCompare(member(b)?.user.email), + }, + { + title: 'Level', + key: 'level', + width: 0, + render: function LevelRender(_, { access_level, organization_member }) { + return ( +
+ + void updateAccessControlMembers([{ member: organization_member, level }]) + } + /> +
+ ) + }, + }, + { + key: 'remove', + width: 0, + render: (_, { organization_member }) => { + return ( + + void updateAccessControlMembers([{ member: organization_member, level: null }]) + } + /> + ) + }, + }, + ] + + return ( +
+ { + if (guardAvailableFeature(AvailableFeature.PROJECT_BASED_PERMISSIONING)) { + await updateAccessControlMembers(newValues.map((member) => ({ member, level }))) + } + }} + options={addableMembers.map((member) => ({ + key: member.id, + label: `${member.user.first_name} ${member.user.email}`, + labelComponent: , + }))} + /> + + +
+ ) +} + +function AccessControlObjectRoles(): JSX.Element | null { + const { accessControlRoles, accessControlsLoading, addableRoles, rolesById, availableLevels } = + useValues(accessControlLogic) + const { updateAccessControlRoles } = useAsyncActions(accessControlLogic) + const { guardAvailableFeature } = useValues(upgradeModalLogic) + + const columns: LemonTableColumns = [ + { + title: 'Role', + key: 'role', + width: 0, + render: (_, { role }) => ( + + + + ), + }, + { + title: 'Members', + key: 'members', + render: (_, { role }) => { + return ( + ({ + email: member.user.email, + name: member.user.first_name, + title: `${member.user.first_name} <${member.user.email}>`, + })) ?? [] + } + /> + ) + }, + }, + { + title: 'Level', + key: 'level', + width: 0, + render: (_, { access_level, role }) => { + return ( +
+ void updateAccessControlRoles([{ role, level }])} + /> +
+ ) + }, + }, + { + key: 'remove', + width: 0, + render: (_, { role }) => { + return ( + void updateAccessControlRoles([{ role, level: null }])} + /> + ) + }, + }, + ] + + return ( +
+ { + if (guardAvailableFeature(AvailableFeature.PROJECT_BASED_PERMISSIONING)) { + await updateAccessControlRoles(newValues.map((role) => ({ role, level }))) + } + }} + options={addableRoles.map((role) => ({ + key: role.id, + label: role.name, + }))} + /> + + +
+ ) +} + +// function LevelComponent(member: FusedTeamMemberType): JSX.Element | null { +// const { user } = useValues(userLogic) +// const { currentTeam } = useValues(teamLogic) +// const { changeUserAccessLevel } = useActions(teamMembersLogic) + +// const myMembershipLevel = isAuthenticatedTeam(currentTeam) ? currentTeam.effective_membership_level : null + +// if (!user) { +// return null +// } + +// const isImplicit = member.organization_level >= OrganizationMembershipLevel.Admin +// const levelName = membershipLevelToName.get(member.level) ?? `unknown (${member.level})` + +// const allowedLevels = teamMembershipLevelIntegers.filter( +// (listLevel) => !getReasonForAccessLevelChangeProhibition(myMembershipLevel, user, member, listLevel) +// ) + +// const possibleOptions = member.explicit_team_level +// ? allowedLevels.concat([member.explicit_team_level]) +// : allowedLevels + +// const disallowedReason = isImplicit +// ? `This user is a member of the project implicitly due to being an organization ${levelName}.` +// : getReasonForAccessLevelChangeProhibition(myMembershipLevel, user, member, allowedLevels) + +// return disallowedReason ? ( +// +// +// {member.level === OrganizationMembershipLevel.Owner && } +// {levelName} +// +// +// ) : ( +// { +// if (listLevel !== null) { +// changeUserAccessLevel(member.user, listLevel) +// } +// }} +// options={possibleOptions.map( +// (listLevel) => +// ({ +// value: listLevel, +// disabled: listLevel === member.explicit_team_level, +// label: +// listLevel > member.level +// ? membershipLevelToName.get(listLevel) +// : membershipLevelToName.get(listLevel), +// } as LemonSelectOption) +// )} +// value={member.explicit_team_level} +// /> +// ) +// } + +function SimplLevelComponent(props: { + size?: LemonSelectProps['size'] + level: AccessControlType['access_level'] | null + levels: AccessControlType['access_level'][] + onChange: (newValue: AccessControlType['access_level']) => void +}): JSX.Element | null { + const { canEditAccessControls } = useValues(accessControlLogic) + + return ( + props.onChange(newValue)} + disabledReason={!canEditAccessControls ? 'You cannot edit this' : undefined} + options={props.levels.map((level) => ({ + value: level, + label: capitalizeFirstLetter(level ?? ''), + }))} + /> + ) +} + +function RemoveAccessButton({ + onConfirm, + subject, +}: { + onConfirm: () => void + subject: 'member' | 'role' +}): JSX.Element { + const { canEditAccessControls } = useValues(accessControlLogic) + + return ( + } + status="danger" + size="small" + disabledReason={!canEditAccessControls ? 'You cannot edit this' : undefined} + onClick={() => + LemonDialog.open({ + title: 'Remove access', + content: `Are you sure you want to remove this ${subject}'s explicit access?`, + primaryButton: { + children: 'Remove', + status: 'danger', + onClick: () => onConfirm(), + }, + }) + } + /> + ) +} + +function AddItemsControls(props: { + placeholder: string + onAdd: (newValues: string[], level: AccessControlType['access_level']) => Promise + options: { + key: string + label: string + }[] +}): JSX.Element | null { + const { availableLevels, canEditAccessControls } = useValues(accessControlLogic) + // TODO: Move this into a form logic + const [items, setItems] = useState([]) + const [level, setLevel] = useState(availableLevels[0] ?? null) + + useEffect(() => { + setLevel(availableLevels[0] ?? null) + }, [availableLevels]) + + const onSubmit = + items.length && level + ? (): void => + void props.onAdd(items, level).then(() => { + setItems([]) + setLevel(availableLevels[0] ?? null) + }) + : undefined + + return ( +
+
+ setItems(newValues)} + mode="multiple" + options={props.options} + disabled={!canEditAccessControls} + /> +
+ + + + Add + +
+ ) +} diff --git a/frontend/src/layout/navigation-3000/sidepanel/panels/access_control/RolesAndResourceAccessControls.tsx b/frontend/src/layout/navigation-3000/sidepanel/panels/access_control/RolesAndResourceAccessControls.tsx new file mode 100644 index 0000000000000..cc198a16c0fbd --- /dev/null +++ b/frontend/src/layout/navigation-3000/sidepanel/panels/access_control/RolesAndResourceAccessControls.tsx @@ -0,0 +1,319 @@ +import { + LemonButton, + LemonDialog, + LemonInput, + LemonInputSelect, + LemonModal, + LemonSelect, + LemonTable, + LemonTableColumns, + ProfileBubbles, + ProfilePicture, +} from '@posthog/lemon-ui' +import { useActions, useValues } from 'kea' +import { capitalizeFirstLetter, Form } from 'kea-forms' +import { PayGateMini } from 'lib/components/PayGateMini/PayGateMini' +import { usersLemonSelectOptions } from 'lib/components/UserSelectItem' +import { LemonField } from 'lib/lemon-ui/LemonField' +import { LemonTableLink } from 'lib/lemon-ui/LemonTable/LemonTableLink' +import { fullName } from 'lib/utils' +import { useMemo, useState } from 'react' +import { userLogic } from 'scenes/userLogic' + +import { AvailableFeature } from '~/types' + +import { roleBasedAccessControlLogic, RoleWithResourceAccessControls } from './roleBasedAccessControlLogic' + +export type RolesAndResourceAccessControlsProps = { + noAccessControls?: boolean +} + +export function RolesAndResourceAccessControls({ noAccessControls }: RolesAndResourceAccessControlsProps): JSX.Element { + const { + rolesWithResourceAccessControls, + rolesLoading, + roleBasedAccessControlsLoading, + resources, + availableLevels, + selectedRoleId, + defaultAccessLevel, + } = useValues(roleBasedAccessControlLogic) + + const { updateRoleBasedAccessControls, selectRoleId, setEditingRoleId } = useActions(roleBasedAccessControlLogic) + + const roleColumns = noAccessControls + ? [] + : resources.map((resource) => ({ + title: resource.replace(/_/g, ' ') + 's', + key: resource, + width: 0, + render: (_: any, { accessControlByResource, role }: RoleWithResourceAccessControls) => { + const ac = accessControlByResource[resource] + + return ( + + updateRoleBasedAccessControls([ + { + resource, + role: role?.id ?? null, + access_level: newValue, + }, + ]) + } + options={availableLevels.map((level) => ({ + value: level, + label: capitalizeFirstLetter(level ?? ''), + }))} + /> + ) + }, + })) + + const columns: LemonTableColumns = [ + { + title: 'Role', + key: 'role', + width: 0, + render: (_, { role }) => ( + + (role.id === selectedRoleId ? selectRoleId(null) : selectRoleId(role.id)) + : undefined + } + title={role?.name ?? 'Default'} + /> + + ), + }, + { + title: 'Members', + key: 'members', + render: (_, { role }) => { + return role ? ( + role.members.length ? ( + ({ + email: member.user.email, + name: member.user.first_name, + title: `${member.user.first_name} <${member.user.email}>`, + }))} + onClick={() => (role.id === selectedRoleId ? selectRoleId(null) : selectRoleId(role.id))} + /> + ) : ( + 'No members' + ) + ) : ( + 'All members' + ) + }, + }, + + ...roleColumns, + ] + + return ( +
+

+ Edit organizational default permission levels for PostHog resources. Use roles to group your + organization members and assign them permissions. +

+ + +
+ !!selectedRoleId && role?.id === selectedRoleId, + onRowExpand: ({ role }) => (role ? selectRoleId(role.id) : undefined), + onRowCollapse: () => selectRoleId(null), + expandedRowRender: ({ role }) => (role ? : null), + rowExpandable: ({ role }) => !!role, + }} + /> + + setEditingRoleId('new')}> + Create role + + +
+
+
+ ) +} + +function RoleDetails({ roleId }: { roleId: string }): JSX.Element | null { + const { user } = useValues(userLogic) + const { sortedMembers, roles } = useValues(roleBasedAccessControlLogic) + const { addMembersToRole, removeMemberFromRole, setEditingRoleId } = useActions(roleBasedAccessControlLogic) + const [membersToAdd, setMembersToAdd] = useState([]) + + const role = roles?.find((role) => role.id === roleId) + + const onSubmit = membersToAdd.length + ? () => { + role && addMembersToRole(role, membersToAdd) + setMembersToAdd([]) + } + : undefined + + const membersNotInRole = useMemo(() => { + const membersInRole = new Set(role?.members.map((member) => member.user.uuid)) + return sortedMembers?.filter((member) => !membersInRole.has(member.user.uuid)) ?? [] + }, [role?.members, sortedMembers]) + + if (!role) { + // This is mostly for typing + return null + } + + return ( +
+
+
+
+ setMembersToAdd(newValues)} + mode="multiple" + options={usersLemonSelectOptions( + membersNotInRole.map((member) => member.user), + 'uuid' + )} + /> +
+ + + Add members + +
+
+ setEditingRoleId(role.id)}> + Edit role + +
+
+ + + }, + width: 32, + }, + { + title: 'Name', + key: 'user_name', + render: (_, member) => + member.user.uuid == user?.uuid ? `${fullName(member.user)} (you)` : fullName(member.user), + sorter: (a, b) => fullName(a.user).localeCompare(fullName(b.user)), + }, + { + title: 'Email', + key: 'user_email', + render: (_, member) => { + return <>{member.user.email} + }, + sorter: (a, b) => a.user.email.localeCompare(b.user.email), + }, + { + key: 'actions', + width: 0, + render: (_, member) => { + return ( +
+ removeMemberFromRole(role, member.id)} + > + Remove + +
+ ) + }, + }, + /* {isAdminOrOwner && deleteMember && ( + } + onClick={() => deleteMember(member.id)} + tooltip="Remove user from role" + type="tertiary" + size="small" + /> + )} */ + ]} + dataSource={role.members} + /> +
+ ) +} + +function RoleModal(): JSX.Element { + const { editingRoleId } = useValues(roleBasedAccessControlLogic) + const { setEditingRoleId, submitEditingRole, deleteRole } = useActions(roleBasedAccessControlLogic) + const isEditing = editingRoleId !== 'new' + + const onDelete = (): void => { + LemonDialog.open({ + title: 'Delete role', + content: 'Are you sure you want to delete this role? This action cannot be undone.', + primaryButton: { + children: 'Delete permanently', + onClick: () => deleteRole(editingRoleId as string), + status: 'danger', + }, + secondaryButton: { + children: 'Cancel', + }, + }) + } + + return ( +
+ +
+ {isEditing ? ( + onDelete()}> + Delete + + ) : null} +
+ + setEditingRoleId(null)}> + Cancel + + + + {!isEditing ? 'Create' : 'Save'} + + + } + > + + + +
+
+ ) +} diff --git a/frontend/src/layout/navigation-3000/sidepanel/panels/access_control/SidePanelAccessControl.tsx b/frontend/src/layout/navigation-3000/sidepanel/panels/access_control/SidePanelAccessControl.tsx new file mode 100644 index 0000000000000..266b012ebcd77 --- /dev/null +++ b/frontend/src/layout/navigation-3000/sidepanel/panels/access_control/SidePanelAccessControl.tsx @@ -0,0 +1,25 @@ +import { useValues } from 'kea' + +import { SidePanelPaneHeader } from '../../components/SidePanelPaneHeader' +import { sidePanelContextLogic } from '../sidePanelContextLogic' +import { AccessControlObject } from './AccessControlObject' + +export const SidePanelAccessControl = (): JSX.Element => { + const { sceneSidePanelContext } = useValues(sidePanelContextLogic) + + return ( +
+ +
+ {sceneSidePanelContext.access_control_resource && sceneSidePanelContext.access_control_resource_id ? ( + + ) : ( +

Not supported

+ )} +
+
+ ) +} diff --git a/frontend/src/layout/navigation-3000/sidepanel/panels/access_control/accessControlLogic.ts b/frontend/src/layout/navigation-3000/sidepanel/panels/access_control/accessControlLogic.ts new file mode 100644 index 0000000000000..1bd16f9b0d724 --- /dev/null +++ b/frontend/src/layout/navigation-3000/sidepanel/panels/access_control/accessControlLogic.ts @@ -0,0 +1,238 @@ +import { LemonSelectOption } from '@posthog/lemon-ui' +import { actions, afterMount, connect, kea, key, listeners, path, props, selectors } from 'kea' +import { loaders } from 'kea-loaders' +import api from 'lib/api' +import { upgradeModalLogic } from 'lib/components/UpgradeModal/upgradeModalLogic' +import { membersLogic } from 'scenes/organization/membersLogic' +import { teamLogic } from 'scenes/teamLogic' + +import { + AccessControlResponseType, + AccessControlType, + AccessControlTypeMember, + AccessControlTypeProject, + AccessControlTypeRole, + AccessControlUpdateType, + APIScopeObject, + OrganizationMemberType, + RoleType, +} from '~/types' + +import type { accessControlLogicType } from './accessControlLogicType' +import { roleBasedAccessControlLogic } from './roleBasedAccessControlLogic' + +export type AccessControlLogicProps = { + resource: APIScopeObject + resource_id: string +} + +export const accessControlLogic = kea([ + props({} as AccessControlLogicProps), + key((props) => `${props.resource}-${props.resource_id}`), + path((key) => ['scenes', 'accessControl', 'accessControlLogic', key]), + connect({ + values: [ + membersLogic, + ['sortedMembers'], + teamLogic, + ['currentTeam'], + roleBasedAccessControlLogic, + ['roles'], + upgradeModalLogic, + ['guardAvailableFeature'], + ], + actions: [membersLogic, ['ensureAllMembersLoaded']], + }), + actions({ + updateAccessControl: ( + accessControl: Pick + ) => ({ accessControl }), + updateAccessControlDefault: (level: AccessControlType['access_level']) => ({ + level, + }), + updateAccessControlRoles: ( + accessControls: { + role: RoleType['id'] + level: AccessControlType['access_level'] + }[] + ) => ({ accessControls }), + updateAccessControlMembers: ( + accessControls: { + member: OrganizationMemberType['id'] + level: AccessControlType['access_level'] + }[] + ) => ({ accessControls }), + }), + loaders(({ values }) => ({ + accessControls: [ + null as AccessControlResponseType | null, + { + loadAccessControls: async () => { + const response = await api.get(values.endpoint) + return response + }, + + updateAccessControlDefault: async ({ level }) => { + await api.put(values.endpoint, { + access_level: level, + }) + + return values.accessControls + }, + + updateAccessControlRoles: async ({ accessControls }) => { + for (const { role, level } of accessControls) { + await api.put(values.endpoint, { + role: role, + access_level: level, + }) + } + + return values.accessControls + }, + + updateAccessControlMembers: async ({ accessControls }) => { + for (const { member, level } of accessControls) { + await api.put(values.endpoint, { + organization_member: member, + access_level: level, + }) + } + + return values.accessControls + }, + }, + ], + })), + listeners(({ actions }) => ({ + updateAccessControlDefaultSuccess: () => actions.loadAccessControls(), + updateAccessControlRolesSuccess: () => actions.loadAccessControls(), + updateAccessControlMembersSuccess: () => actions.loadAccessControls(), + })), + selectors({ + endpoint: [ + () => [(_, props) => props], + (props): string => { + // TODO: This is far from perfect... but it's a start + if (props.resource === 'project') { + return `api/projects/@current/access_controls` + } + return `api/projects/@current/${props.resource}s/${props.resource_id}/access_controls` + }, + ], + humanReadableResource: [ + () => [(_, props) => props], + (props): string => { + return props.resource.replace(/_/g, ' ') + }, + ], + + availableLevelsWithNone: [ + (s) => [s.accessControls], + (accessControls): string[] => { + return accessControls?.available_access_levels ?? [] + }, + ], + + availableLevels: [ + (s) => [s.availableLevelsWithNone], + (availableLevelsWithNone): string[] => { + return availableLevelsWithNone.filter((level) => level !== 'none') + }, + ], + + canEditAccessControls: [ + (s) => [s.accessControls], + (accessControls): boolean | null => { + return accessControls?.user_can_edit_access_levels ?? null + }, + ], + + accessControlDefaultLevel: [ + (s) => [s.accessControls], + (accessControls): string | null => { + return accessControls?.default_access_level ?? null + }, + ], + + accessControlDefaultOptions: [ + (s) => [s.availableLevelsWithNone, (_, props) => props.resource], + (availableLevelsWithNone): LemonSelectOption[] => { + const options = availableLevelsWithNone.map((level) => ({ + value: level, + // TODO: Correct "a" and "an" + label: level === 'none' ? 'No access by default' : `Everyone is a ${level} by default`, + })) + + return options + }, + ], + accessControlDefault: [ + (s) => [s.accessControls, s.accessControlDefaultLevel], + (accessControls, accessControlDefaultLevel): AccessControlTypeProject => { + const found = accessControls?.access_controls?.find( + (accessControl) => !accessControl.organization_member && !accessControl.role + ) as AccessControlTypeProject + return ( + found ?? { + access_level: accessControlDefaultLevel, + } + ) + }, + ], + + accessControlMembers: [ + (s) => [s.accessControls], + (accessControls): AccessControlTypeMember[] => { + return (accessControls?.access_controls || []).filter( + (accessControl) => !!accessControl.organization_member + ) as AccessControlTypeMember[] + }, + ], + + accessControlRoles: [ + (s) => [s.accessControls], + (accessControls): AccessControlTypeRole[] => { + return (accessControls?.access_controls || []).filter( + (accessControl) => !!accessControl.role + ) as AccessControlTypeRole[] + }, + ], + + rolesById: [ + (s) => [s.roles], + (roles): Record => { + return Object.fromEntries((roles || []).map((role) => [role.id, role])) + }, + ], + + addableRoles: [ + (s) => [s.roles, s.accessControlRoles], + (roles, accessControlRoles): RoleType[] => { + return roles ? roles.filter((role) => !accessControlRoles.find((ac) => ac.role === role.id)) : [] + }, + ], + + membersById: [ + (s) => [s.sortedMembers], + (members): Record => { + return Object.fromEntries((members || []).map((member) => [member.id, member])) + }, + ], + + addableMembers: [ + (s) => [s.sortedMembers, s.accessControlMembers], + (members, accessControlMembers): any[] => { + return members + ? members.filter( + (member) => !accessControlMembers.find((ac) => ac.organization_member === member.id) + ) + : [] + }, + ], + }), + afterMount(({ actions }) => { + actions.loadAccessControls() + actions.ensureAllMembersLoaded() + }), +]) diff --git a/frontend/src/layout/navigation-3000/sidepanel/panels/access_control/roleBasedAccessControlLogic.ts b/frontend/src/layout/navigation-3000/sidepanel/panels/access_control/roleBasedAccessControlLogic.ts new file mode 100644 index 0000000000000..42294d39469f5 --- /dev/null +++ b/frontend/src/layout/navigation-3000/sidepanel/panels/access_control/roleBasedAccessControlLogic.ts @@ -0,0 +1,259 @@ +import { lemonToast } from '@posthog/lemon-ui' +import { actions, afterMount, connect, kea, listeners, path, reducers, selectors } from 'kea' +import { forms } from 'kea-forms' +import { loaders } from 'kea-loaders' +import { actionToUrl, router } from 'kea-router' +import api from 'lib/api' +import { membersLogic } from 'scenes/organization/membersLogic' +import { teamLogic } from 'scenes/teamLogic' +import { userLogic } from 'scenes/userLogic' + +import { + AccessControlResponseType, + AccessControlType, + AccessControlTypeRole, + AccessControlUpdateType, + APIScopeObject, + AvailableFeature, + RoleType, +} from '~/types' + +import type { roleBasedAccessControlLogicType } from './roleBasedAccessControlLogicType' + +export type RoleWithResourceAccessControls = { + role?: RoleType + accessControlByResource: Record +} + +export const roleBasedAccessControlLogic = kea([ + path(['scenes', 'accessControl', 'roleBasedAccessControlLogic']), + connect({ + values: [membersLogic, ['sortedMembers'], teamLogic, ['currentTeam'], userLogic, ['hasAvailableFeature']], + actions: [membersLogic, ['ensureAllMembersLoaded']], + }), + actions({ + updateRoleBasedAccessControls: ( + accessControls: Pick[] + ) => ({ accessControls }), + selectRoleId: (roleId: RoleType['id'] | null) => ({ roleId }), + deleteRole: (roleId: RoleType['id']) => ({ roleId }), + removeMemberFromRole: (role: RoleType, roleMemberId: string) => ({ role, roleMemberId }), + addMembersToRole: (role: RoleType, members: string[]) => ({ role, members }), + setEditingRoleId: (roleId: string | null) => ({ roleId }), + }), + reducers({ + selectedRoleId: [ + null as string | null, + { + selectRoleId: (_, { roleId }) => roleId, + }, + ], + editingRoleId: [ + null as string | null, + { + setEditingRoleId: (_, { roleId }) => roleId, + }, + ], + }), + loaders(({ values }) => ({ + roleBasedAccessControls: [ + null as AccessControlResponseType | null, + { + loadRoleBasedAccessControls: async () => { + const response = await api.get( + 'api/projects/@current/global_access_controls' + ) + return response + }, + + updateRoleBasedAccessControls: async ({ accessControls }) => { + for (const control of accessControls) { + await api.put('api/projects/@current/global_access_controls', { + ...control, + }) + } + + return values.roleBasedAccessControls + }, + }, + ], + roles: [ + null as RoleType[] | null, + { + loadRoles: async () => { + const response = await api.roles.list() + return response?.results || [] + }, + addMembersToRole: async ({ role, members }) => { + if (!values.roles) { + return null + } + const newMembers = await Promise.all( + members.map(async (userUuid: string) => await api.roles.members.create(role.id, userUuid)) + ) + + role.members = [...role.members, ...newMembers] + + return [...values.roles] + }, + removeMemberFromRole: async ({ role, roleMemberId }) => { + if (!values.roles) { + return null + } + await api.roles.members.delete(role.id, roleMemberId) + role.members = role.members.filter((roleMember) => roleMember.id !== roleMemberId) + return [...values.roles] + }, + deleteRole: async ({ roleId }) => { + const role = values.roles?.find((r) => r.id === roleId) + if (!role) { + return values.roles + } + await api.roles.delete(role.id) + lemonToast.success(`Role "${role.name}" deleted`) + return values.roles?.filter((r) => r.id !== role.id) || [] + }, + }, + ], + })), + + forms(({ values, actions }) => ({ + editingRole: { + defaults: { + name: '', + }, + errors: ({ name }) => { + return { + name: !name ? 'Please choose a name for the role' : null, + } + }, + submit: async ({ name }) => { + if (!values.editingRoleId) { + return + } + let role: RoleType | null = null + if (values.editingRoleId === 'new') { + role = await api.roles.create(name) + } else { + role = await api.roles.update(values.editingRoleId, { name }) + } + + actions.loadRoles() + actions.setEditingRoleId(null) + actions.selectRoleId(role.id) + }, + }, + })), + + listeners(({ actions, values }) => ({ + updateRoleBasedAccessControlsSuccess: () => actions.loadRoleBasedAccessControls(), + loadRolesSuccess: () => { + if (router.values.hashParams.role) { + actions.selectRoleId(router.values.hashParams.role) + } + }, + deleteRoleSuccess: () => { + actions.loadRoles() + actions.setEditingRoleId(null) + actions.selectRoleId(null) + }, + + setEditingRoleId: () => { + const existingRole = values.roles?.find((role) => role.id === values.editingRoleId) + actions.resetEditingRole({ + name: existingRole?.name || '', + }) + }, + })), + + selectors({ + availableLevels: [ + (s) => [s.roleBasedAccessControls], + (roleBasedAccessControls): string[] => { + return roleBasedAccessControls?.available_access_levels ?? [] + }, + ], + + defaultAccessLevel: [ + (s) => [s.roleBasedAccessControls], + (roleBasedAccessControls): string | null => { + return roleBasedAccessControls?.default_access_level ?? null + }, + ], + defaultResourceAccessControls: [ + (s) => [s.roleBasedAccessControls], + (roleBasedAccessControls): RoleWithResourceAccessControls => { + const accessControls = roleBasedAccessControls?.access_controls ?? [] + + // Find all acs without a roles (they are the default ones) + const accessControlByResource = accessControls + .filter((control) => !control.role) + .reduce( + (acc, control) => ({ + ...acc, + [control.resource]: control, + }), + {} as Record + ) + + return { accessControlByResource } + }, + ], + rolesWithResourceAccessControls: [ + (s) => [s.roles, s.roleBasedAccessControls, s.defaultResourceAccessControls], + (roles, roleBasedAccessControls, defaultResourceAccessControls): RoleWithResourceAccessControls[] => { + if (!roles) { + return [] + } + + const accessControls = roleBasedAccessControls?.access_controls ?? [] + + return [ + defaultResourceAccessControls, + ...roles.map((role) => { + const accessControlByResource = accessControls + .filter((control) => control.role === role.id) + .reduce( + (acc, control) => ({ + ...acc, + [control.resource]: control, + }), + {} as Record + ) + + return { role, accessControlByResource } + }), + ] + }, + ], + + resources: [ + () => [], + (): AccessControlType['resource'][] => { + // TODO: Sync this as an enum + return ['feature_flag', 'dashboard', 'insight', 'session_recording', 'plugin'] + }, + ], + }), + afterMount(({ actions, values }) => { + if (values.hasAvailableFeature(AvailableFeature.ROLE_BASED_ACCESS)) { + actions.loadRoles() + actions.loadRoleBasedAccessControls() + actions.ensureAllMembersLoaded() + } + }), + + actionToUrl(({ values }) => ({ + selectRoleId: () => { + const { currentLocation } = router.values + return [ + currentLocation.pathname, + currentLocation.searchParams, + { + ...currentLocation.hashParams, + role: values.selectedRoleId ?? undefined, + }, + ] + }, + })), +]) diff --git a/frontend/src/layout/navigation-3000/sidepanel/panels/activity/sidePanelActivityLogic.tsx b/frontend/src/layout/navigation-3000/sidepanel/panels/activity/sidePanelActivityLogic.tsx index e2a235881da6a..4bb158035fad1 100644 --- a/frontend/src/layout/navigation-3000/sidepanel/panels/activity/sidePanelActivityLogic.tsx +++ b/frontend/src/layout/navigation-3000/sidepanel/panels/activity/sidePanelActivityLogic.tsx @@ -10,7 +10,10 @@ import { toParams } from 'lib/utils' import posthog from 'posthog-js' import { teamLogic } from 'scenes/teamLogic' -import { ActivityFilters, activityForSceneLogic } from './activityForSceneLogic' +import { ActivityScope, UserBasicType } from '~/types' + +import { SidePanelSceneContext } from '../../types' +import { sidePanelContextLogic } from '../sidePanelContextLogic' import type { sidePanelActivityLogicType } from './sidePanelActivityLogicType' const POLL_TIMEOUT = 5 * 60 * 1000 @@ -31,10 +34,16 @@ export enum SidePanelActivityTab { All = 'all', } +export type ActivityFilters = { + scope?: ActivityScope + item_id?: ActivityLogItem['item_id'] + user?: UserBasicType['id'] +} + export const sidePanelActivityLogic = kea([ path(['scenes', 'navigation', 'sidepanel', 'sidePanelActivityLogic']), connect({ - values: [activityForSceneLogic, ['sceneActivityFilters']], + values: [sidePanelContextLogic, ['sceneSidePanelContext']], }), actions({ togglePolling: (pageIsVisible: boolean) => ({ pageIsVisible }), @@ -252,8 +261,16 @@ export const sidePanelActivityLogic = kea([ }), subscriptions(({ actions, values }) => ({ - sceneActivityFilters: (activityFilters) => { - actions.setFiltersForCurrentPage(activityFilters ? { ...values.filters, ...activityFilters } : null) + sceneSidePanelContext: (sceneSidePanelContext: SidePanelSceneContext) => { + actions.setFiltersForCurrentPage( + sceneSidePanelContext + ? { + ...values.filters, + scope: sceneSidePanelContext.activity_scope, + item_id: sceneSidePanelContext.activity_item_id, + } + : null + ) }, filters: () => { if (values.activeTab === SidePanelActivityTab.All) { @@ -265,7 +282,7 @@ export const sidePanelActivityLogic = kea([ afterMount(({ actions, values }) => { actions.loadImportantChanges() - const activityFilters = values.sceneActivityFilters + const activityFilters = values.sceneSidePanelContext actions.setFiltersForCurrentPage(activityFilters ? { ...values.filters, ...activityFilters } : null) }), diff --git a/frontend/src/layout/navigation-3000/sidepanel/panels/discussion/sidePanelDiscussionLogic.ts b/frontend/src/layout/navigation-3000/sidepanel/panels/discussion/sidePanelDiscussionLogic.ts index 5793deba3469f..9d1ba1d536d9b 100644 --- a/frontend/src/layout/navigation-3000/sidepanel/panels/discussion/sidePanelDiscussionLogic.ts +++ b/frontend/src/layout/navigation-3000/sidepanel/panels/discussion/sidePanelDiscussionLogic.ts @@ -6,7 +6,7 @@ import { FEATURE_FLAGS } from 'lib/constants' import { featureFlagLogic } from 'lib/logic/featureFlagLogic' import { CommentsLogicProps } from 'scenes/comments/commentsLogic' -import { activityForSceneLogic } from '../activity/activityForSceneLogic' +import { sidePanelContextLogic } from '../sidePanelContextLogic' import type { sidePanelDiscussionLogicType } from './sidePanelDiscussionLogicType' export const sidePanelDiscussionLogic = kea([ @@ -16,7 +16,7 @@ export const sidePanelDiscussionLogic = kea([ resetCommentCount: true, }), connect({ - values: [featureFlagLogic, ['featureFlags'], activityForSceneLogic, ['sceneActivityFilters']], + values: [featureFlagLogic, ['featureFlags'], sidePanelContextLogic, ['sceneSidePanelContext']], }), loaders(({ values }) => ({ commentCount: [ @@ -45,12 +45,12 @@ export const sidePanelDiscussionLogic = kea([ selectors({ commentsLogicProps: [ - (s) => [s.sceneActivityFilters], - (activityFilters): CommentsLogicProps | null => { - return activityFilters?.scope + (s) => [s.sceneSidePanelContext], + (sceneSidePanelContext): CommentsLogicProps | null => { + return sceneSidePanelContext.activity_scope ? { - scope: activityFilters.scope, - item_id: activityFilters.item_id, + scope: sceneSidePanelContext.activity_scope, + item_id: sceneSidePanelContext.activity_item_id, } : null }, diff --git a/frontend/src/layout/navigation-3000/sidepanel/panels/exports/sidePanelExportsLogic.ts b/frontend/src/layout/navigation-3000/sidepanel/panels/exports/sidePanelExportsLogic.ts index c9107c4ac695f..8f26e5927842e 100644 --- a/frontend/src/layout/navigation-3000/sidepanel/panels/exports/sidePanelExportsLogic.ts +++ b/frontend/src/layout/navigation-3000/sidepanel/panels/exports/sidePanelExportsLogic.ts @@ -1,23 +1,14 @@ import { afterMount, connect, kea, path } from 'kea' import { exportsLogic } from 'lib/components/ExportButton/exportsLogic' -import { featureFlagLogic } from 'lib/logic/featureFlagLogic' import { sidePanelStateLogic } from '~/layout/navigation-3000/sidepanel/sidePanelStateLogic' -import { activityForSceneLogic } from '../activity/activityForSceneLogic' import type { sidePanelExportsLogicType } from './sidePanelExportsLogicType' export const sidePanelExportsLogic = kea([ path(['scenes', 'navigation', 'sidepanel', 'sidePanelExportsLogic']), connect({ - values: [ - featureFlagLogic, - ['featureFlags'], - activityForSceneLogic, - ['sceneActivityFilters'], - exportsLogic, - ['exports', 'freshUndownloadedExports'], - ], + values: [exportsLogic, ['exports', 'freshUndownloadedExports']], actions: [sidePanelStateLogic, ['openSidePanel'], exportsLogic, ['loadExports', 'removeFresh']], }), afterMount(({ actions }) => { diff --git a/frontend/src/layout/navigation-3000/sidepanel/panels/activity/activityForSceneLogic.ts b/frontend/src/layout/navigation-3000/sidepanel/panels/sidePanelContextLogic.ts similarity index 57% rename from frontend/src/layout/navigation-3000/sidepanel/panels/activity/activityForSceneLogic.ts rename to frontend/src/layout/navigation-3000/sidepanel/panels/sidePanelContextLogic.ts index 180461c465996..a31f3b5b8451b 100644 --- a/frontend/src/layout/navigation-3000/sidepanel/panels/activity/activityForSceneLogic.ts +++ b/frontend/src/layout/navigation-3000/sidepanel/panels/sidePanelContextLogic.ts @@ -1,22 +1,14 @@ import { connect, kea, path, selectors } from 'kea' import { router } from 'kea-router' import { objectsEqual } from 'kea-test-utils' -import { ActivityLogItem } from 'lib/components/ActivityLog/humanizeActivity' import { removeProjectIdIfPresent } from 'lib/utils/router-utils' import { sceneLogic } from 'scenes/sceneLogic' import { SceneConfig } from 'scenes/sceneTypes' -import { ActivityScope, UserBasicType } from '~/types' +import { SIDE_PANEL_CONTEXT_KEY, SidePanelSceneContext } from '../types' +import type { sidePanelContextLogicType } from './sidePanelContextLogicType' -import type { activityForSceneLogicType } from './activityForSceneLogicType' - -export type ActivityFilters = { - scope?: ActivityScope - item_id?: ActivityLogItem['item_id'] - user?: UserBasicType['id'] -} - -export const activityFiltersForScene = (sceneConfig: SceneConfig | null): ActivityFilters | null => { +const activityFiltersForScene = (sceneConfig: SceneConfig | null): SidePanelSceneContext | null => { if (sceneConfig?.activityScope) { // NOTE: - HACKY, we are just parsing the item_id from the url optimistically... const pathParts = removeProjectIdIfPresent(router.values.currentLocation.pathname).split('/') @@ -24,37 +16,42 @@ export const activityFiltersForScene = (sceneConfig: SceneConfig | null): Activi // Loose check for the item_id being a number, a short_id (8 chars) or a uuid if (item_id && (item_id.length === 8 || item_id.length === 36 || !isNaN(parseInt(item_id)))) { - return { scope: sceneConfig.activityScope, item_id } + return { activity_scope: sceneConfig.activityScope, activity_item_id: item_id } } - return { scope: sceneConfig.activityScope } + return { activity_scope: sceneConfig.activityScope } } return null } -export const activityForSceneLogic = kea([ - path(['scenes', 'navigation', 'sidepanel', 'activityForSceneLogic']), +export const sidePanelContextLogic = kea([ + path(['scenes', 'navigation', 'sidepanel', 'sidePanelContextLogic']), connect({ values: [sceneLogic, ['sceneConfig']], }), selectors({ - sceneActivityFilters: [ + sceneSidePanelContext: [ (s) => [ + s.sceneConfig, // Similar to "breadcrumbs" (state, props) => { const activeSceneLogic = sceneLogic.selectors.activeSceneLogic(state, props) - const sceneConfig = s.sceneConfig(state, props) - if (activeSceneLogic && 'activityFilters' in activeSceneLogic.selectors) { + if (activeSceneLogic && SIDE_PANEL_CONTEXT_KEY in activeSceneLogic.selectors) { const activeLoadedScene = sceneLogic.selectors.activeLoadedScene(state, props) - return activeSceneLogic.selectors.activityFilters( + return activeSceneLogic.selectors[SIDE_PANEL_CONTEXT_KEY]( state, activeLoadedScene?.paramsToProps?.(activeLoadedScene?.sceneParams) || props ) } - return activityFiltersForScene(sceneConfig) + return null }, ], - (filters): ActivityFilters | null => filters, + (sceneConfig, context): SidePanelSceneContext => { + return { + ...(context ?? {}), + ...(!context?.activity_scope ? activityFiltersForScene(sceneConfig) : {}), + } + }, { equalityCheck: objectsEqual }, ], }), diff --git a/frontend/src/layout/navigation-3000/sidepanel/sidePanelLogic.tsx b/frontend/src/layout/navigation-3000/sidepanel/sidePanelLogic.tsx index 2a4add974a1d9..071b5715dd9ba 100644 --- a/frontend/src/layout/navigation-3000/sidepanel/sidePanelLogic.tsx +++ b/frontend/src/layout/navigation-3000/sidepanel/sidePanelLogic.tsx @@ -9,6 +9,7 @@ import { activationLogic } from '~/layout/navigation-3000/sidepanel/panels/activ import { AvailableFeature, SidePanelTab } from '~/types' import { sidePanelActivityLogic } from './panels/activity/sidePanelActivityLogic' +import { sidePanelContextLogic } from './panels/sidePanelContextLogic' import { sidePanelStatusLogic } from './panels/sidePanelStatusLogic' import type { sidePanelLogicType } from './sidePanelLogicType' import { sidePanelStateLogic } from './sidePanelStateLogic' @@ -38,6 +39,8 @@ export const sidePanelLogic = kea([ ['unreadCount'], sidePanelStatusLogic, ['status'], + sidePanelContextLogic, + ['sceneSidePanelContext'], userLogic, ['hasAvailableFeature'], router, @@ -48,8 +51,8 @@ export const sidePanelLogic = kea([ selectors({ enabledTabs: [ - (s) => [s.isCloudOrDev, s.isReady, s.hasCompletedAllTasks, s.featureFlags], - (isCloudOrDev, isReady, hasCompletedAllTasks, featureflags) => { + (s) => [s.isCloudOrDev, s.isReady, s.hasCompletedAllTasks, s.featureFlags, s.sceneSidePanelContext], + (isCloudOrDev, isReady, hasCompletedAllTasks, featureflags, sceneSidePanelContext) => { const tabs: SidePanelTab[] = [] tabs.push(SidePanelTab.ExperimentFeatureFlag) @@ -73,6 +76,14 @@ export const sidePanelLogic = kea([ tabs.push(SidePanelTab.Status) } + if ( + featureflags[FEATURE_FLAGS.ACCESS_CONTROL] && + sceneSidePanelContext.access_control_resource && + sceneSidePanelContext.access_control_resource_id + ) { + tabs.push(SidePanelTab.AccessControl) + } + return tabs }, ], diff --git a/frontend/src/layout/navigation-3000/sidepanel/types.ts b/frontend/src/layout/navigation-3000/sidepanel/types.ts new file mode 100644 index 0000000000000..28da07acb1c89 --- /dev/null +++ b/frontend/src/layout/navigation-3000/sidepanel/types.ts @@ -0,0 +1,12 @@ +import { ActivityLogItem } from 'lib/components/ActivityLog/humanizeActivity' + +import { ActivityScope, APIScopeObject } from '~/types' + +/** Allows scenes to set a context which enables richer features of the side panel */ +export type SidePanelSceneContext = { + access_control_resource?: APIScopeObject + access_control_resource_id?: string + activity_scope?: ActivityScope + activity_item_id?: ActivityLogItem['item_id'] +} +export const SIDE_PANEL_CONTEXT_KEY = 'sidePanelContext' diff --git a/frontend/src/lib/api.mock.ts b/frontend/src/lib/api.mock.ts index 9902182ca8e0f..813061c27138f 100644 --- a/frontend/src/lib/api.mock.ts +++ b/frontend/src/lib/api.mock.ts @@ -84,6 +84,7 @@ export const MOCK_DEFAULT_TEAM: TeamType = { autocapture_web_vitals_opt_in: false, autocapture_exceptions_errors_to_ignore: [], effective_membership_level: OrganizationMembershipLevel.Admin, + user_access_level: 'admin', access_control: true, has_group_types: true, primary_dashboard: 1, diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index 31933112d7b9a..6e1adbf7e1d2c 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -901,6 +901,10 @@ class ApiRequest { return await api.update(this.assembleFullUrl(), options?.data, options) } + public async put(options?: ApiMethodOptions & { data: any }): Promise { + return await api.put(this.assembleFullUrl(), options?.data, options) + } + public async create(options?: ApiMethodOptions & { data: any }): Promise { return await api.create(this.assembleFullUrl(), options?.data, options) } @@ -2495,14 +2499,19 @@ const api = { }) }, - async update(url: string, data: any, options?: ApiMethodOptions): Promise { + async _update( + method: 'PATCH' | 'PUT', + url: string, + data: P, + options?: ApiMethodOptions + ): Promise { url = prepareUrl(url) ensureProjectIdNotInvalid(url) const isFormData = data instanceof FormData - const response = await handleFetch(url, 'PATCH', async () => { + const response = await handleFetch(url, method, async () => { return await fetch(url, { - method: 'PATCH', + method, headers: { ...objectClean(options?.headers ?? {}), ...(isFormData ? {} : { 'Content-Type': 'application/json' }), @@ -2517,7 +2526,15 @@ const api = { return await getJSONOrNull(response) }, - async create(url: string, data?: any, options?: ApiMethodOptions): Promise { + async update(url: string, data: P, options?: ApiMethodOptions): Promise { + return api._update('PATCH', url, data, options) + }, + + async put(url: string, data: P, options?: ApiMethodOptions): Promise { + return api._update('PUT', url, data, options) + }, + + async create(url: string, data?: P, options?: ApiMethodOptions): Promise { const res = await api.createResponse(url, data, options) return await getJSONOrNull(res) }, diff --git a/frontend/src/lib/components/RestrictedArea.tsx b/frontend/src/lib/components/RestrictedArea.tsx index ade847740c42a..8cd3633679d88 100644 --- a/frontend/src/lib/components/RestrictedArea.tsx +++ b/frontend/src/lib/components/RestrictedArea.tsx @@ -37,6 +37,7 @@ export function useRestrictedArea({ scope, minimumAccessLevel }: UseRestrictedAr if (!isAuthenticatedTeam(currentTeam)) { return 'Loading current project…' } + // TODO: Check the user_access_level scopeAccessLevel = currentTeam.effective_membership_level } else { if (!currentOrganization) { diff --git a/frontend/src/lib/components/UserSelectItem.tsx b/frontend/src/lib/components/UserSelectItem.tsx index 903440db365e4..80ba10d09f690 100644 --- a/frontend/src/lib/components/UserSelectItem.tsx +++ b/frontend/src/lib/components/UserSelectItem.tsx @@ -9,9 +9,9 @@ export interface UserSelectItemProps { export function UserSelectItem({ user }: UserSelectItemProps): JSX.Element { return ( - + - + {user.first_name} {`<${user.email}>`} diff --git a/frontend/src/lib/constants.tsx b/frontend/src/lib/constants.tsx index cb2a0ba0d42e7..3b52d54ef5105 100644 --- a/frontend/src/lib/constants.tsx +++ b/frontend/src/lib/constants.tsx @@ -182,6 +182,7 @@ export const FEATURE_FLAGS = { NEW_EXPERIMENTS_UI: 'new-experiments-ui', // owner: @jurajmajerik #team-feature-success REPLAY_ERROR_CLUSTERING: 'session-replay-error-clustering', // owner: #team-replay AUDIT_LOGS_ACCESS: 'audit-logs-access', // owner: #team-growth + ACCESS_CONTROL: 'access-control', // owner: @benjackwhite SUBSCRIBE_FROM_PAYGATE: 'subscribe-from-paygate', // owner: #team-growth HEATMAPS_UI: 'heatmaps-ui', // owner: @benjackwhite THEME: 'theme', // owner: @aprilfools diff --git a/frontend/src/models/dashboardsModel.tsx b/frontend/src/models/dashboardsModel.tsx index 99c4cdf01ea96..201d8a6076cc3 100644 --- a/frontend/src/models/dashboardsModel.tsx +++ b/frontend/src/models/dashboardsModel.tsx @@ -114,10 +114,10 @@ export const dashboardsModel = kea([ const beforeChange = { ...values.rawDashboards[id] } - const response = (await api.update( + const response = await api.update( `api/environments/${teamLogic.values.currentTeamId}/dashboards/${id}`, payload - )) as DashboardType + ) const updatedAttribute = Object.keys(payload)[0] if (updatedAttribute === 'name' || updatedAttribute === 'description' || updatedAttribute === 'tags') { eventUsageLogic.actions.reportDashboardFrontEndUpdate( @@ -134,10 +134,10 @@ export const dashboardsModel = kea([ button: { label: 'Undo', action: async () => { - const reverted = (await api.update( + const reverted = await api.update( `api/environments/${teamLogic.values.currentTeamId}/dashboards/${id}`, beforeChange - )) as DashboardType + ) actions.updateDashboardSuccess(getQueryBasedDashboard(reverted)) lemonToast.success('Dashboard change reverted') }, @@ -160,31 +160,34 @@ export const dashboardsModel = kea([ }) ) as DashboardType, pinDashboard: async ({ id, source }) => { - const response = (await api.update( + const response = await api.update( `api/environments/${teamLogic.values.currentTeamId}/dashboards/${id}`, { pinned: true, } - )) as DashboardType + ) eventUsageLogic.actions.reportDashboardPinToggled(true, source) return getQueryBasedDashboard(response)! }, unpinDashboard: async ({ id, source }) => { - const response = (await api.update( + const response = await api.update( `api/environments/${teamLogic.values.currentTeamId}/dashboards/${id}`, { pinned: false, } - )) as DashboardType + ) eventUsageLogic.actions.reportDashboardPinToggled(false, source) return getQueryBasedDashboard(response)! }, duplicateDashboard: async ({ id, name, show, duplicateTiles }) => { - const result = (await api.create(`api/environments/${teamLogic.values.currentTeamId}/dashboards/`, { - use_dashboard: id, - name: `${name} (Copy)`, - duplicate_tiles: duplicateTiles, - })) as DashboardType + const result = await api.create( + `api/environments/${teamLogic.values.currentTeamId}/dashboards/`, + { + use_dashboard: id, + name: `${name} (Copy)`, + duplicate_tiles: duplicateTiles, + } + ) if (show) { router.actions.push(urls.dashboard(result.id)) } diff --git a/frontend/src/scenes/authentication/login2FALogic.ts b/frontend/src/scenes/authentication/login2FALogic.ts index 4da5deb50adbe..f15e21b899b41 100644 --- a/frontend/src/scenes/authentication/login2FALogic.ts +++ b/frontend/src/scenes/authentication/login2FALogic.ts @@ -69,7 +69,7 @@ export const login2FALogic = kea([ submit: async ({ token }, breakpoint) => { breakpoint() try { - return await api.create('api/login/token', { token }) + return await api.create('api/login/token', { token }) } catch (e) { const { code } = e as Record const { detail } = e as Record diff --git a/frontend/src/scenes/authentication/loginLogic.ts b/frontend/src/scenes/authentication/loginLogic.ts index 19d73edc00158..b51fae8e4ece2 100644 --- a/frontend/src/scenes/authentication/loginLogic.ts +++ b/frontend/src/scenes/authentication/loginLogic.ts @@ -102,7 +102,7 @@ export const loginLogic = kea([ submit: async ({ email, password }, breakpoint) => { breakpoint() try { - return await api.create('api/login', { email, password }) + return await api.create('api/login', { email, password }) } catch (e) { const { code } = e as Record let { detail } = e as Record diff --git a/frontend/src/scenes/authentication/setup2FALogic.ts b/frontend/src/scenes/authentication/setup2FALogic.ts index 7fe843a95fdaf..eb415d47a7381 100644 --- a/frontend/src/scenes/authentication/setup2FALogic.ts +++ b/frontend/src/scenes/authentication/setup2FALogic.ts @@ -58,7 +58,7 @@ export const setup2FALogic = kea([ submit: async ({ token }, breakpoint) => { breakpoint() try { - return await api.create('api/users/@me/validate_2fa/', { token }) + return await api.create('api/users/@me/validate_2fa/', { token }) } catch (e) { const { code } = e as Record const { detail } = e as Record diff --git a/frontend/src/scenes/dashboard/DashboardCollaborators.tsx b/frontend/src/scenes/dashboard/DashboardCollaborators.tsx index 048d668bc71fd..daf3bda0d161e 100644 --- a/frontend/src/scenes/dashboard/DashboardCollaborators.tsx +++ b/frontend/src/scenes/dashboard/DashboardCollaborators.tsx @@ -3,6 +3,7 @@ import { useActions, useValues } from 'kea' import { PayGateMini } from 'lib/components/PayGateMini/PayGateMini' import { usersLemonSelectOptions } from 'lib/components/UserSelectItem' import { DashboardPrivilegeLevel, DashboardRestrictionLevel, privilegeLevelToName } from 'lib/constants' +import { useFeatureFlag } from 'lib/hooks/useFeatureFlag' import { LemonBanner } from 'lib/lemon-ui/LemonBanner' import { LemonButton } from 'lib/lemon-ui/LemonButton' import { LemonInputSelect } from 'lib/lemon-ui/LemonInputSelect/LemonInputSelect' @@ -11,6 +12,7 @@ import { ProfilePicture } from 'lib/lemon-ui/ProfilePicture' import { Tooltip } from 'lib/lemon-ui/Tooltip' import { dashboardLogic } from 'scenes/dashboard/dashboardLogic' +import { AccessControlObject } from '~/layout/navigation-3000/sidepanel/panels/access_control/AccessControlObject' import { AvailableFeature, DashboardType, FusedDashboardCollaboratorType, UserType } from '~/types' import { dashboardCollaboratorsLogic } from './dashboardCollaboratorsLogic' @@ -37,68 +39,77 @@ export function DashboardCollaboration({ dashboardId }: { dashboardId: Dashboard dashboardCollaboratorsLogic({ dashboardId }) ) + const newAccessControl = useFeatureFlag('ACCESS_CONTROL') + return ( dashboard && ( <> - {(!canEditDashboard || !canRestrictDashboard) && ( - - {canEditDashboard - ? "You aren't allowed to change the restriction level – only the dashboard owner and project admins can." - : "You aren't allowed to change sharing settings – only dashboard collaborators with edit settings can."} - - )} - - triggerDashboardUpdate({ - restriction_level: newValue, - }) - } - options={DASHBOARD_RESTRICTION_OPTIONS} - loading={dashboardLoading} - fullWidth - disabled={!canRestrictDashboard} - /> - {dashboard.restriction_level > DashboardRestrictionLevel.EveryoneInProjectCanEdit && ( -
-
Collaborators
- {canEditDashboard && ( -
-
- - setExplicitCollaboratorsToBeAdded(newValues) - } - mode="multiple" - data-attr="subscribed-emails" - options={usersLemonSelectOptions(addableMembers, 'uuid')} - /> + {newAccessControl ? ( + + ) : ( + <> + {(!canEditDashboard || !canRestrictDashboard) && ( + + {canEditDashboard + ? "You aren't allowed to change the restriction level – only the dashboard owner and project admins can." + : "You aren't allowed to change sharing settings – only dashboard collaborators with edit settings can."} + + )} + + triggerDashboardUpdate({ + restriction_level: newValue, + }) + } + options={DASHBOARD_RESTRICTION_OPTIONS} + loading={dashboardLoading} + fullWidth + disabled={!canRestrictDashboard} + /> + {dashboard.restriction_level > DashboardRestrictionLevel.EveryoneInProjectCanEdit && ( +
+
Collaborators
+ {canEditDashboard && ( +
+
+ + setExplicitCollaboratorsToBeAdded(newValues) + } + mode="multiple" + options={usersLemonSelectOptions(addableMembers, 'uuid')} + /> +
+ addExplicitCollaborators()} + > + Add + +
+ )} +
Project members with access
+
+ {allCollaborators.map((collaborator) => ( + + ))}
- addExplicitCollaborators()} - > - Add -
)} -
Project members with access
-
- {allCollaborators.map((collaborator) => ( - - ))} -
-
+ )} diff --git a/frontend/src/scenes/dashboard/dashboardLogic.tsx b/frontend/src/scenes/dashboard/dashboardLogic.tsx index 4addf1f04f4c0..0b6236931cf2b 100644 --- a/frontend/src/scenes/dashboard/dashboardLogic.tsx +++ b/frontend/src/scenes/dashboard/dashboardLogic.tsx @@ -30,6 +30,7 @@ import { Scene } from 'scenes/sceneTypes' import { urls } from 'scenes/urls' import { userLogic } from 'scenes/userLogic' +import { SIDE_PANEL_CONTEXT_KEY, SidePanelSceneContext } from '~/layout/navigation-3000/sidepanel/types' import { dashboardsModel } from '~/models/dashboardsModel' import { insightsModel } from '~/models/insightsModel' import { variableDataLogic } from '~/queries/nodes/DataVisualization/Components/Variables/variableDataLogic' @@ -38,6 +39,7 @@ import { getQueryBasedDashboard, getQueryBasedInsightModel } from '~/queries/nod import { pollForResults } from '~/queries/query' import { DashboardFilter, DataVisualizationNode, HogQLVariable, NodeKind, RefreshType } from '~/queries/schema' import { + ActivityScope, AnyPropertyFilter, Breadcrumb, DashboardLayoutSize, @@ -991,6 +993,21 @@ export const dashboardLogic = kea([ }, ], ], + + [SIDE_PANEL_CONTEXT_KEY]: [ + (s) => [s.dashboard], + (dashboard): SidePanelSceneContext | null => { + return dashboard + ? { + activity_scope: ActivityScope.DASHBOARD, + activity_item_id: `${dashboard.id}`, + access_control_resource: 'dashboard', + access_control_resource_id: `${dashboard.id}`, + } + : null + }, + ], + sortTilesByLayout: [ (s) => [s.layoutForItem], (layoutForItem) => (tiles: Array) => { diff --git a/frontend/src/scenes/data-warehouse/saved_queries/dataWarehouseViewsLogic.tsx b/frontend/src/scenes/data-warehouse/saved_queries/dataWarehouseViewsLogic.tsx index 10df0ed6c1f8b..56e600ccb3985 100644 --- a/frontend/src/scenes/data-warehouse/saved_queries/dataWarehouseViewsLogic.tsx +++ b/frontend/src/scenes/data-warehouse/saved_queries/dataWarehouseViewsLogic.tsx @@ -29,7 +29,7 @@ export const dataWarehouseViewsLogic = kea([ const savedQueries = await api.dataWarehouseSavedQueries.list() if (router.values.location.pathname.includes(urls.dataModel()) && !cache.pollingInterval) { - cache.pollingInterval = setInterval(actions.loadDataWarehouseSavedQueries, 5000) + cache.pollingInterval = setInterval(() => actions.loadDataWarehouseSavedQueries(), 5000) } else { clearInterval(cache.pollingInterval) } diff --git a/frontend/src/scenes/feature-flags/FeatureFlag.tsx b/frontend/src/scenes/feature-flags/FeatureFlag.tsx index c440c80286283..714cbfe93b646 100644 --- a/frontend/src/scenes/feature-flags/FeatureFlag.tsx +++ b/frontend/src/scenes/feature-flags/FeatureFlag.tsx @@ -10,7 +10,6 @@ import { CopyToClipboardInline } from 'lib/components/CopyToClipboard' import { NotFound } from 'lib/components/NotFound' import { ObjectTags } from 'lib/components/ObjectTags/ObjectTags' import { PageHeader } from 'lib/components/PageHeader' -import { PayGateMini } from 'lib/components/PayGateMini/PayGateMini' import { FEATURE_FLAGS } from 'lib/constants' import { LemonBanner } from 'lib/lemon-ui/LemonBanner' import { LemonButton } from 'lib/lemon-ui/LemonButton' @@ -31,11 +30,11 @@ import { useEffect, useState } from 'react' import { Dashboard } from 'scenes/dashboard/Dashboard' import { dashboardLogic } from 'scenes/dashboard/dashboardLogic' import { EmptyDashboardComponent } from 'scenes/dashboard/EmptyDashboardComponent' +import { FeatureFlagPermissions } from 'scenes/feature-flags/FeatureFlagPermissions' import { UTM_TAGS } from 'scenes/feature-flags/FeatureFlagSnippets' import { JSONEditorInput } from 'scenes/feature-flags/JSONEditorInput' import { concatWithPunctuation } from 'scenes/insights/utils' import { NotebookSelectButton } from 'scenes/notebooks/NotebookSelectButton/NotebookSelectButton' -import { ResourcePermission } from 'scenes/ResourcePermissionModal' import { SceneExport } from 'scenes/sceneTypes' import { urls } from 'scenes/urls' import { userLogic } from 'scenes/userLogic' @@ -57,14 +56,12 @@ import { PropertyOperator, QueryBasedInsightModel, ReplayTabs, - Resource, } from '~/types' import { AnalysisTab } from './FeatureFlagAnalysisTab' import { FeatureFlagAutoRollback } from './FeatureFlagAutoRollout' import { FeatureFlagCodeExample } from './FeatureFlagCodeExample' import { featureFlagLogic } from './featureFlagLogic' -import { featureFlagPermissionsLogic } from './featureFlagPermissionsLogic' import FeatureFlagProjects from './FeatureFlagProjects' import { FeatureFlagReleaseConditions } from './FeatureFlagReleaseConditions' import FeatureFlagSchedule from './FeatureFlagSchedule' @@ -108,13 +105,6 @@ export function FeatureFlag({ id }: { id?: string } = {}): JSX.Element { setActiveTab, } = useActions(featureFlagLogic) - const { addableRoles, unfilteredAddableRolesLoading, rolesToAdd, derivedRoles } = useValues( - featureFlagPermissionsLogic({ flagId: featureFlag.id }) - ) - const { setRolesToAdd, addAssociatedRoles, deleteAssociatedRole } = useActions( - featureFlagPermissionsLogic({ flagId: featureFlag.id }) - ) - const { tags } = useValues(tagsModel) const { hasAvailableFeature } = useValues(userLogic) @@ -221,21 +211,7 @@ export function FeatureFlag({ id }: { id?: string } = {}): JSX.Element { tabs.push({ label: 'Permissions', key: FeatureFlagsTab.PERMISSIONS, - content: ( - - setRolesToAdd(roleIds)} - rolesToAdd={rolesToAdd} - addableRoles={addableRoles} - addableRolesLoading={unfilteredAddableRolesLoading} - onAdd={() => addAssociatedRoles()} - roles={derivedRoles} - deleteAssociatedRole={(id) => deleteAssociatedRole({ roleId: id })} - canEdit={featureFlag.can_edit} - /> - - ), + content: , }) } @@ -431,21 +407,7 @@ export function FeatureFlag({ id }: { id?: string } = {}): JSX.Element {

Permissions

- - setRolesToAdd(roleIds)} - rolesToAdd={rolesToAdd} - addableRoles={addableRoles} - addableRolesLoading={unfilteredAddableRolesLoading} - onAdd={() => addAssociatedRoles()} - roles={derivedRoles} - deleteAssociatedRole={(id) => - deleteAssociatedRole({ roleId: id }) - } - canEdit={featureFlag.can_edit} - /> - +
diff --git a/frontend/src/scenes/feature-flags/FeatureFlagCodeInstructions.stories.tsx b/frontend/src/scenes/feature-flags/FeatureFlagCodeInstructions.stories.tsx index 0c5bc5df8edff..ca03d6721f834 100644 --- a/frontend/src/scenes/feature-flags/FeatureFlagCodeInstructions.stories.tsx +++ b/frontend/src/scenes/feature-flags/FeatureFlagCodeInstructions.stories.tsx @@ -31,6 +31,7 @@ const REGULAR_FEATURE_FLAG: FeatureFlagType = { can_edit: true, tags: [], surveys: [], + user_access_level: 'editor', } const GROUP_FEATURE_FLAG: FeatureFlagType = { diff --git a/frontend/src/scenes/ResourcePermissionModal.tsx b/frontend/src/scenes/feature-flags/FeatureFlagPermissions.tsx similarity index 72% rename from frontend/src/scenes/ResourcePermissionModal.tsx rename to frontend/src/scenes/feature-flags/FeatureFlagPermissions.tsx index b7361519f398d..ab3858345a75d 100644 --- a/frontend/src/scenes/ResourcePermissionModal.tsx +++ b/frontend/src/scenes/feature-flags/FeatureFlagPermissions.tsx @@ -1,15 +1,19 @@ import { IconGear, IconTrash } from '@posthog/icons' -import { LemonButton, LemonModal, LemonTable } from '@posthog/lemon-ui' -import { useValues } from 'kea' +import { LemonButton, LemonTable } from '@posthog/lemon-ui' +import { useActions, useValues } from 'kea' +import { PayGateMini } from 'lib/components/PayGateMini/PayGateMini' import { TitleWithIcon } from 'lib/components/TitleWithIcon' +import { useFeatureFlag } from 'lib/hooks/useFeatureFlag' import { LemonInputSelect, LemonInputSelectOption } from 'lib/lemon-ui/LemonInputSelect/LemonInputSelect' import { LemonTableColumns } from 'lib/lemon-ui/LemonTable' -import { AccessLevel, Resource, RoleType } from '~/types' +import { AccessControlObject } from '~/layout/navigation-3000/sidepanel/panels/access_control/AccessControlObject' +import { AccessLevel, AvailableFeature, FeatureFlagType, Resource, RoleType } from '~/types' -import { permissionsLogic } from './settings/organization/Permissions/permissionsLogic' -import { rolesLogic } from './settings/organization/Permissions/Roles/rolesLogic' -import { urls } from './urls' +import { permissionsLogic } from '../settings/organization/Permissions/permissionsLogic' +import { rolesLogic } from '../settings/organization/Permissions/Roles/rolesLogic' +import { urls } from '../urls' +import { featureFlagPermissionsLogic } from './featureFlagPermissionsLogic' interface ResourcePermissionProps { addableRoles: RoleType[] @@ -23,13 +27,7 @@ interface ResourcePermissionProps { canEdit: boolean } -interface ResourcePermissionModalProps extends ResourcePermissionProps { - title: string - visible: boolean - onClose: () => void -} - -export function roleLemonSelectOptions(roles: RoleType[]): LemonInputSelectOption[] { +function roleLemonSelectOptions(roles: RoleType[]): LemonInputSelectOption[] { return roles.map((role) => ({ key: role.id, label: `${role.name}`, @@ -41,35 +39,37 @@ export function roleLemonSelectOptions(roles: RoleType[]): LemonInputSelectOptio })) } -export function ResourcePermissionModal({ - title, - visible, - onClose, - rolesToAdd, - addableRoles, - onChange, - addableRolesLoading, - onAdd, - roles, - deleteAssociatedRole, - canEdit, -}: ResourcePermissionModalProps): JSX.Element { +export function FeatureFlagPermissions({ featureFlag }: { featureFlag: FeatureFlagType }): JSX.Element { + const { addableRoles, unfilteredAddableRolesLoading, rolesToAdd, derivedRoles } = useValues( + featureFlagPermissionsLogic({ flagId: featureFlag.id }) + ) + const { setRolesToAdd, addAssociatedRoles, deleteAssociatedRole } = useActions( + featureFlagPermissionsLogic({ flagId: featureFlag.id }) + ) + + const newAccessControls = useFeatureFlag('ACCESS_CONTROL') + + if (newAccessControls) { + if (!featureFlag.id) { + return

Not supported

+ } + return + } + return ( - <> - - - - + + setRolesToAdd(roleIds)} + rolesToAdd={rolesToAdd} + addableRoles={addableRoles} + addableRolesLoading={unfilteredAddableRolesLoading} + onAdd={() => addAssociatedRoles()} + roles={derivedRoles} + deleteAssociatedRole={(id) => deleteAssociatedRole({ roleId: id })} + canEdit={featureFlag.can_edit} + /> + ) } diff --git a/frontend/src/scenes/feature-flags/activityDescriptions.tsx b/frontend/src/scenes/feature-flags/activityDescriptions.tsx index 93fec0692b0c3..a85f73cde21ad 100644 --- a/frontend/src/scenes/feature-flags/activityDescriptions.tsx +++ b/frontend/src/scenes/feature-flags/activityDescriptions.tsx @@ -252,6 +252,7 @@ const featureFlagActionsMapping: Record< analytics_dashboards: () => null, has_enriched_analytics: () => null, surveys: () => null, + user_access_level: () => null, } export function flagActivityDescriber(logItem: ActivityLogItem, asNotification?: boolean): HumanizedChange { diff --git a/frontend/src/scenes/feature-flags/featureFlagLogic.ts b/frontend/src/scenes/feature-flags/featureFlagLogic.ts index 65026ccfd3453..18234d1d2ae98 100644 --- a/frontend/src/scenes/feature-flags/featureFlagLogic.ts +++ b/frontend/src/scenes/feature-flags/featureFlagLogic.ts @@ -23,9 +23,11 @@ import { urls } from 'scenes/urls' import { userLogic } from 'scenes/userLogic' import { sidePanelStateLogic } from '~/layout/navigation-3000/sidepanel/sidePanelStateLogic' +import { SIDE_PANEL_CONTEXT_KEY, SidePanelSceneContext } from '~/layout/navigation-3000/sidepanel/types' import { groupsModel } from '~/models/groupsModel' import { getQueryBasedInsightModel } from '~/queries/nodes/InsightViz/utils' import { + ActivityScope, AvailableFeature, Breadcrumb, CohortType, @@ -96,6 +98,7 @@ const NEW_FLAG: FeatureFlagType = { surveys: null, performed_rollback: false, can_edit: true, + user_access_level: 'editor', tags: [], } const NEW_VARIANT = { @@ -880,6 +883,19 @@ export const featureFlagLogic = kea([ { key: [Scene.FeatureFlag, featureFlag.id || 'unknown'], name: featureFlag.key || 'Unnamed' }, ], ], + [SIDE_PANEL_CONTEXT_KEY]: [ + (s) => [s.featureFlag], + (featureFlag): SidePanelSceneContext | null => { + return featureFlag?.id + ? { + activity_scope: ActivityScope.DASHBOARD, + activity_item_id: `${featureFlag.id}`, + access_control_resource: 'feature_flag', + access_control_resource_id: `${featureFlag.id}`, + } + : null + }, + ], filteredDashboards: [ (s) => [s.dashboards, s.featureFlag], (dashboards, featureFlag) => { diff --git a/frontend/src/scenes/insights/insightSceneLogic.tsx b/frontend/src/scenes/insights/insightSceneLogic.tsx index 3f79ace2432d9..ab58df3d1ec86 100644 --- a/frontend/src/scenes/insights/insightSceneLogic.tsx +++ b/frontend/src/scenes/insights/insightSceneLogic.tsx @@ -15,7 +15,7 @@ import { teamLogic } from 'scenes/teamLogic' import { mathsLogic } from 'scenes/trends/mathsLogic' import { urls } from 'scenes/urls' -import { ActivityFilters } from '~/layout/navigation-3000/sidepanel/panels/activity/activityForSceneLogic' +import { SIDE_PANEL_CONTEXT_KEY, SidePanelSceneContext } from '~/layout/navigation-3000/sidepanel/types' import { cohortsModel } from '~/models/cohortsModel' import { groupsModel } from '~/models/groupsModel' import { getDefaultQuery } from '~/queries/nodes/InsightViz/utils' @@ -210,13 +210,15 @@ export const insightSceneLogic = kea([ ] }, ], - activityFilters: [ + [SIDE_PANEL_CONTEXT_KEY]: [ (s) => [s.insight], - (insight): ActivityFilters | null => { - return insight + (insight): SidePanelSceneContext | null => { + return insight?.id ? { - scope: ActivityScope.INSIGHT, - item_id: `${insight.id}`, + activity_scope: ActivityScope.INSIGHT, + activity_item_id: `${insight.id}`, + access_control_resource: 'insight', + access_control_resource_id: `${insight.id}`, } : null }, diff --git a/frontend/src/scenes/instance/SystemStatus/staffUsersLogic.ts b/frontend/src/scenes/instance/SystemStatus/staffUsersLogic.ts index 51054ab2fa8ac..3ac34a80e733b 100644 --- a/frontend/src/scenes/instance/SystemStatus/staffUsersLogic.ts +++ b/frontend/src/scenes/instance/SystemStatus/staffUsersLogic.ts @@ -33,8 +33,7 @@ export const staffUsersLogic = kea([ actions.setStaffUsersToBeAdded([]) const newStaffUsers = await Promise.all( staffUsersToBeAdded.map( - async (userUuid) => - (await api.update(`api/users/${userUuid}`, { is_staff: true })) as UserType + async (userUuid) => await api.update(`api/users/${userUuid}`, { is_staff: true }) ) ) const updatedAllUsers: UserType[] = [ @@ -45,7 +44,7 @@ export const staffUsersLogic = kea([ return updatedAllUsers }, deleteStaffUser: async ({ userUuid }) => { - await api.update(`api/users/${userUuid}`, { is_staff: false }) + await api.update(`api/users/${userUuid}`, { is_staff: false }) if (values.user?.uuid === userUuid) { actions.loadUser() // Loads the main user object to properly reflect staff user changes router.actions.push(urls.projectHomepage()) diff --git a/frontend/src/scenes/notebooks/Notebook/NotebookShare.tsx b/frontend/src/scenes/notebooks/Notebook/NotebookShare.tsx index 1a9233289616c..4cc7b8c8b6130 100644 --- a/frontend/src/scenes/notebooks/Notebook/NotebookShare.tsx +++ b/frontend/src/scenes/notebooks/Notebook/NotebookShare.tsx @@ -1,6 +1,7 @@ import { IconCopy } from '@posthog/icons' import { LemonBanner, LemonButton, LemonDivider } from '@posthog/lemon-ui' import { useValues } from 'kea' +import { FlaggedFeature } from 'lib/components/FlaggedFeature' import { LemonDialog } from 'lib/lemon-ui/LemonDialog' import { base64Encode } from 'lib/utils' import { copyToClipboard } from 'lib/utils/copyToClipboard' @@ -8,6 +9,8 @@ import posthog from 'posthog-js' import { useState } from 'react' import { urls } from 'scenes/urls' +import { AccessControlObject } from '~/layout/navigation-3000/sidepanel/panels/access_control/AccessControlObject' + import { notebookLogic } from './notebookLogic' export type NotebookShareProps = { @@ -26,7 +29,13 @@ export function NotebookShare({ shortId }: NotebookShareProps): JSX.Element { } return ( -
+
+ + <> + + + +

Internal Link

{!isLocalOnly ? ( <> diff --git a/frontend/src/scenes/notebooks/Notebook/__mocks__/notebook-12345.json b/frontend/src/scenes/notebooks/Notebook/__mocks__/notebook-12345.json index f2ac6bd3c8d16..4e31800d43919 100644 --- a/frontend/src/scenes/notebooks/Notebook/__mocks__/notebook-12345.json +++ b/frontend/src/scenes/notebooks/Notebook/__mocks__/notebook-12345.json @@ -59,5 +59,6 @@ "first_name": "Paul", "email": "paul@posthog.com", "is_email_verified": false - } + }, + "user_access_level": "editor" } diff --git a/frontend/src/scenes/notebooks/Notebook/__mocks__/notebook-template-for-snapshot.ts b/frontend/src/scenes/notebooks/Notebook/__mocks__/notebook-template-for-snapshot.ts index 175647c05a7a6..13c7ac4fdd08b 100644 --- a/frontend/src/scenes/notebooks/Notebook/__mocks__/notebook-template-for-snapshot.ts +++ b/frontend/src/scenes/notebooks/Notebook/__mocks__/notebook-template-for-snapshot.ts @@ -15,6 +15,7 @@ export const notebookTestTemplate = ( created_by: MOCK_DEFAULT_BASIC_USER, last_modified_by: MOCK_DEFAULT_BASIC_USER, version: 1, + user_access_level: 'editor' as const, content: { type: 'doc', content: [ diff --git a/frontend/src/scenes/notebooks/Notebook/migrations/migrate.test.ts b/frontend/src/scenes/notebooks/Notebook/migrations/migrate.test.ts index d1409e1716f6d..2dd7b6aa1915d 100644 --- a/frontend/src/scenes/notebooks/Notebook/migrations/migrate.test.ts +++ b/frontend/src/scenes/notebooks/Notebook/migrations/migrate.test.ts @@ -933,10 +933,12 @@ describe('migrate()', () => { it.each(contentToExpected)('migrates %s', (_name, prevContent, nextContent) => { const prevNotebook: NotebookType = { ...mockNotebook, + user_access_level: 'editor' as const, content: { type: 'doc', content: prevContent }, } const nextNotebook: NotebookType = { ...mockNotebook, + user_access_level: 'editor' as const, content: { type: 'doc', content: nextContent }, } diff --git a/frontend/src/scenes/notebooks/Notebook/notebookLogic.ts b/frontend/src/scenes/notebooks/Notebook/notebookLogic.ts index 5b9396ecdc94b..2b9468526f4f6 100644 --- a/frontend/src/scenes/notebooks/Notebook/notebookLogic.ts +++ b/frontend/src/scenes/notebooks/Notebook/notebookLogic.ts @@ -232,6 +232,7 @@ export const notebookLogic = kea([ content: null, text_content: null, version: 0, + user_access_level: 'editor', } } else if (props.shortId.startsWith('template-')) { response = @@ -442,8 +443,9 @@ export const notebookLogic = kea([ ], isEditable: [ - (s) => [s.shouldBeEditable, s.previewContent], - (shouldBeEditable, previewContent) => shouldBeEditable && !previewContent, + (s) => [s.shouldBeEditable, s.previewContent, s.notebook], + (shouldBeEditable, previewContent, notebook) => + shouldBeEditable && !previewContent && notebook?.user_access_level === 'editor', ], }), listeners(({ values, actions, cache }) => ({ @@ -517,6 +519,11 @@ export const notebookLogic = kea([ ) }, setLocalContent: async ({ updateEditor, jsonContent }, breakpoint) => { + if (values.notebook?.user_access_level !== 'editor') { + actions.clearLocalContent() + return + } + if (values.previewContent) { // We don't want to modify the content if we are viewing a preview return diff --git a/frontend/src/scenes/notebooks/NotebookMenu.tsx b/frontend/src/scenes/notebooks/NotebookMenu.tsx index 9cea5e74fbe37..db6f97b69539b 100644 --- a/frontend/src/scenes/notebooks/NotebookMenu.tsx +++ b/frontend/src/scenes/notebooks/NotebookMenu.tsx @@ -39,6 +39,10 @@ export function NotebookMenu({ shortId }: NotebookLogicProps): JSX.Element { label: 'Delete', icon: , status: 'danger', + disabledReason: + notebook?.user_access_level !== 'editor' + ? 'You do not have permission to delete this notebook.' + : undefined, onClick: () => { notebooksModel.actions.deleteNotebook(shortId, notebook?.title) diff --git a/frontend/src/scenes/notebooks/NotebookTemplates/notebookTemplates.ts b/frontend/src/scenes/notebooks/NotebookTemplates/notebookTemplates.ts index f1c63fe133674..1bda77931daa8 100644 --- a/frontend/src/scenes/notebooks/NotebookTemplates/notebookTemplates.ts +++ b/frontend/src/scenes/notebooks/NotebookTemplates/notebookTemplates.ts @@ -737,6 +737,7 @@ export const LOCAL_NOTEBOOK_TEMPLATES: NotebookType[] = [ }, ], }, + user_access_level: 'viewer' as const, }, ].map((template) => ({ ...template, diff --git a/frontend/src/scenes/notebooks/notebookSceneLogic.ts b/frontend/src/scenes/notebooks/notebookSceneLogic.ts index 592a1b39e09ed..6d987f3a780a4 100644 --- a/frontend/src/scenes/notebooks/notebookSceneLogic.ts +++ b/frontend/src/scenes/notebooks/notebookSceneLogic.ts @@ -2,8 +2,9 @@ import { afterMount, connect, kea, key, path, props, selectors } from 'kea' import { Scene } from 'scenes/sceneTypes' import { urls } from 'scenes/urls' +import { SIDE_PANEL_CONTEXT_KEY, SidePanelSceneContext } from '~/layout/navigation-3000/sidepanel/types' import { notebooksModel } from '~/models/notebooksModel' -import { Breadcrumb } from '~/types' +import { ActivityScope, Breadcrumb } from '~/types' import { notebookLogic } from './Notebook/notebookLogic' import type { notebookSceneLogicType } from './notebookSceneLogicType' @@ -16,7 +17,12 @@ export const notebookSceneLogic = kea([ props({} as NotebookSceneLogicProps), key(({ shortId }) => shortId), connect((props: NotebookSceneLogicProps) => ({ - values: [notebookLogic(props), ['notebook', 'notebookLoading'], notebooksModel, ['notebooksLoading']], + values: [ + notebookLogic(props), + ['notebook', 'notebookLoading', 'isLocalOnly'], + notebooksModel, + ['notebooksLoading'], + ], actions: [notebookLogic(props), ['loadNotebook'], notebooksModel, ['createNotebook']], })), selectors(() => ({ @@ -41,6 +47,20 @@ export const notebookSceneLogic = kea([ }, ], ], + + [SIDE_PANEL_CONTEXT_KEY]: [ + (s) => [s.notebookId, s.isLocalOnly], + (notebookId, isLocalOnly): SidePanelSceneContext | null => { + return notebookId && !isLocalOnly + ? { + activity_scope: ActivityScope.NOTEBOOK, + activity_item_id: notebookId, + access_control_resource: 'notebook', + access_control_resource_id: notebookId, + } + : null + }, + ], })), afterMount(({ actions, props }) => { diff --git a/frontend/src/scenes/persons/personsLogic.tsx b/frontend/src/scenes/persons/personsLogic.tsx index 7d92c0593eb54..4db8da312f344 100644 --- a/frontend/src/scenes/persons/personsLogic.tsx +++ b/frontend/src/scenes/persons/personsLogic.tsx @@ -13,7 +13,7 @@ import { Scene } from 'scenes/sceneTypes' import { teamLogic } from 'scenes/teamLogic' import { urls } from 'scenes/urls' -import { ActivityFilters } from '~/layout/navigation-3000/sidepanel/panels/activity/activityForSceneLogic' +import { SIDE_PANEL_CONTEXT_KEY, SidePanelSceneContext } from '~/layout/navigation-3000/sidepanel/types' import { hogqlQuery } from '~/queries/query' import { ActivityScope, @@ -256,13 +256,13 @@ export const personsLogic = kea([ }, ], - activityFilters: [ + [SIDE_PANEL_CONTEXT_KEY]: [ (s) => [s.person], - (person): ActivityFilters => { + (person): SidePanelSceneContext => { return { - scope: ActivityScope.PERSON, + activity_scope: ActivityScope.PERSON, // TODO: Is this correct? It doesn't seem to work... - item_id: person?.id ? `${person?.id}` : undefined, + activity_item_id: person?.id ? `${person?.id}` : undefined, } }, ], diff --git a/frontend/src/scenes/projectLogic.ts b/frontend/src/scenes/projectLogic.ts index 8712f9d4d8279..069aa44c02ff2 100644 --- a/frontend/src/scenes/projectLogic.ts +++ b/frontend/src/scenes/projectLogic.ts @@ -51,10 +51,7 @@ export const projectLogic = kea([ throw new Error('Current project has not been loaded yet, so it cannot be updated!') } - const patchedProject = (await api.update( - `api/projects/${values.currentProject.id}`, - payload - )) as ProjectType + const patchedProject = await api.update(`api/projects/${values.currentProject.id}`, payload) breakpoint() actions.loadUser() diff --git a/frontend/src/scenes/session-recordings/sessionReplaySceneLogic.ts b/frontend/src/scenes/session-recordings/sessionReplaySceneLogic.ts index 492f65fea4038..b8e83d70ca9c4 100644 --- a/frontend/src/scenes/session-recordings/sessionReplaySceneLogic.ts +++ b/frontend/src/scenes/session-recordings/sessionReplaySceneLogic.ts @@ -6,7 +6,7 @@ import { capitalizeFirstLetter } from 'lib/utils' import { Scene } from 'scenes/sceneTypes' import { urls } from 'scenes/urls' -import { ActivityFilters } from '~/layout/navigation-3000/sidepanel/panels/activity/activityForSceneLogic' +import { SIDE_PANEL_CONTEXT_KEY, SidePanelSceneContext } from '~/layout/navigation-3000/sidepanel/types' import { ActivityScope, Breadcrumb, ReplayTabs } from '~/types' import type { sessionReplaySceneLogicType } from './sessionReplaySceneLogicType' @@ -74,13 +74,13 @@ export const sessionReplaySceneLogic = kea([ return breadcrumbs }, ], - activityFilters: [ + [SIDE_PANEL_CONTEXT_KEY]: [ () => [router.selectors.searchParams], - (searchParams): ActivityFilters | null => { + (searchParams): SidePanelSceneContext | null => { return searchParams.sessionRecordingId ? { - scope: ActivityScope.REPLAY, - item_id: searchParams.sessionRecordingId, + activity_scope: ActivityScope.REPLAY, + activity_item_id: searchParams.sessionRecordingId, } : null }, diff --git a/frontend/src/scenes/settings/SettingsMap.tsx b/frontend/src/scenes/settings/SettingsMap.tsx index e3d49dddd1c23..353121070259f 100644 --- a/frontend/src/scenes/settings/SettingsMap.tsx +++ b/frontend/src/scenes/settings/SettingsMap.tsx @@ -3,6 +3,7 @@ import { PersonsJoinMode } from 'scenes/settings/environment/PersonsJoinMode' import { PersonsOnEvents } from 'scenes/settings/environment/PersonsOnEvents' import { SessionsTableVersion } from 'scenes/settings/environment/SessionsTableVersion' +import { RolesAndResourceAccessControls } from '~/layout/navigation-3000/sidepanel/panels/access_control/RolesAndResourceAccessControls' import { Realm } from '~/types' import { @@ -47,7 +48,7 @@ import { OrganizationDangerZone } from './organization/OrganizationDangerZone' import { OrganizationDisplayName } from './organization/OrgDisplayName' import { OrganizationEmailPreferences } from './organization/OrgEmailPreferences' import { OrganizationLogo } from './organization/OrgLogo' -import { PermissionsGrid } from './organization/Permissions/PermissionsGrid' +import { RoleBasedAccess } from './organization/Permissions/RoleBasedAccess' import { VerifiedDomains } from './organization/VerifiedDomains/VerifiedDomains' import { ProjectDangerZone } from './project/ProjectDangerZone' import { ProjectDisplayName } from './project/ProjectSettings' @@ -279,11 +280,11 @@ export const SETTINGS_MAP: SettingSection[] = [ }, { level: 'environment', - id: 'environment-rbac', + id: 'project-access-control', title: 'Access control', settings: [ { - id: 'environment-rbac', + id: 'project-access-control', title: 'Access control', component: , }, @@ -313,8 +314,15 @@ export const SETTINGS_MAP: SettingSection[] = [ title: 'Display name', component: , }, + { + id: 'project-role-based-access-control', + title: 'Role based access control', + flag: 'ACCESS_CONTROL', + component: , + }, ], }, + { level: 'project', id: 'project-danger-zone', @@ -370,25 +378,25 @@ export const SETTINGS_MAP: SettingSection[] = [ }, { level: 'organization', - id: 'organization-authentication', - title: 'Authentication domains & SSO', + id: 'organization-rbac', + title: 'Role-based access', settings: [ { - id: 'authentication-domains', - title: 'Authentication Domains', - component: , + id: 'organization-rbac', + title: 'Role-based access', + component: , }, ], }, { level: 'organization', - id: 'organization-rbac', - title: 'Role-based access', + id: 'organization-authentication', + title: 'Authentication domains & SSO', settings: [ { - id: 'organization-rbac', - title: 'Role-based access', - component: , + id: 'authentication-domains', + title: 'Authentication Domains', + component: , }, ], }, diff --git a/frontend/src/scenes/settings/environment/TeamAccessControl.tsx b/frontend/src/scenes/settings/environment/TeamAccessControl.tsx index 88cfdf5f2caee..404d51886d162 100644 --- a/frontend/src/scenes/settings/environment/TeamAccessControl.tsx +++ b/frontend/src/scenes/settings/environment/TeamAccessControl.tsx @@ -4,6 +4,7 @@ import { useActions, useValues } from 'kea' import { RestrictionScope, useRestrictedArea } from 'lib/components/RestrictedArea' import { upgradeModalLogic } from 'lib/components/UpgradeModal/upgradeModalLogic' import { OrganizationMembershipLevel, TeamMembershipLevel } from 'lib/constants' +import { useFeatureFlag } from 'lib/hooks/useFeatureFlag' import { IconCancel } from 'lib/lemon-ui/icons' import { LemonDialog } from 'lib/lemon-ui/LemonDialog' import { LemonTableColumns } from 'lib/lemon-ui/LemonTable' @@ -19,6 +20,7 @@ import { organizationLogic } from 'scenes/organizationLogic' import { isAuthenticatedTeam, teamLogic } from 'scenes/teamLogic' import { userLogic } from 'scenes/userLogic' +import { AccessControlObject } from '~/layout/navigation-3000/sidepanel/panels/access_control/AccessControlObject' import { AvailableFeature, FusedTeamMemberType } from '~/types' import { AddMembersModalWithButton } from './AddMembersModal' @@ -154,7 +156,7 @@ export function TeamMembers(): JSX.Element | null { title: 'Name', key: 'user_first_name', render: (_, member) => - member.user.uuid == user.uuid ? `${member.user.first_name} (me)` : member.user.first_name, + member.user.uuid == user.uuid ? `${member.user.first_name} (you)` : member.user.first_name, sorter: (a, b) => a.user.first_name.localeCompare(b.user.first_name), }, { @@ -214,6 +216,12 @@ export function TeamAccessControl(): JSX.Element { minimumAccessLevel: OrganizationMembershipLevel.Admin, }) + const newAccessControl = useFeatureFlag('ACCESS_CONTROL') + + if (newAccessControl) { + return + } + return ( <>

diff --git a/frontend/src/scenes/settings/organization/Members.tsx b/frontend/src/scenes/settings/organization/Members.tsx index cceeaa73e538d..00d8bf08bc8e4 100644 --- a/frontend/src/scenes/settings/organization/Members.tsx +++ b/frontend/src/scenes/settings/organization/Members.tsx @@ -165,7 +165,7 @@ export function Members(): JSX.Element | null { title: 'Name', key: 'user_name', render: (_, member) => - member.user.uuid == user.uuid ? `${fullName(member.user)} (me)` : fullName(member.user), + member.user.uuid == user.uuid ? `${fullName(member.user)} (you)` : fullName(member.user), sorter: (a, b) => fullName(a.user).localeCompare(fullName(b.user)), }, { diff --git a/frontend/src/scenes/settings/organization/Permissions/RoleBasedAccess.tsx b/frontend/src/scenes/settings/organization/Permissions/RoleBasedAccess.tsx new file mode 100644 index 0000000000000..abd6489095f82 --- /dev/null +++ b/frontend/src/scenes/settings/organization/Permissions/RoleBasedAccess.tsx @@ -0,0 +1,12 @@ +// NOTE: This is only to allow testing the new RBAC system + +import { useFeatureFlag } from 'lib/hooks/useFeatureFlag' + +import { RolesAndResourceAccessControls } from '~/layout/navigation-3000/sidepanel/panels/access_control/RolesAndResourceAccessControls' + +import { PermissionsGrid } from './PermissionsGrid' + +export function RoleBasedAccess(): JSX.Element { + const newAccessControl = useFeatureFlag('ACCESS_CONTROL') + return newAccessControl ? : +} diff --git a/frontend/src/scenes/settings/organization/VerifiedDomains/__snapshots__/verifiedDomainsLogic.test.ts.snap b/frontend/src/scenes/settings/organization/VerifiedDomains/__snapshots__/verifiedDomainsLogic.test.ts.snap index 55f4935df8fee..5594078e03993 100644 --- a/frontend/src/scenes/settings/organization/VerifiedDomains/__snapshots__/verifiedDomainsLogic.test.ts.snap +++ b/frontend/src/scenes/settings/organization/VerifiedDomains/__snapshots__/verifiedDomainsLogic.test.ts.snap @@ -113,6 +113,7 @@ exports[`verifiedDomainsLogic values has proper defaults 1`] = ` "test_account_filters_default_checked": false, "timezone": "UTC", "updated_at": "2022-03-17T16:09:21.566253Z", + "user_access_level": "admin", "uuid": "TEAM_UUID", }, ], diff --git a/frontend/src/scenes/settings/organization/VerifiedDomains/verifiedDomainsLogic.ts b/frontend/src/scenes/settings/organization/VerifiedDomains/verifiedDomainsLogic.ts index de997f5d7b1cc..f139a10c18888 100644 --- a/frontend/src/scenes/settings/organization/VerifiedDomains/verifiedDomainsLogic.ts +++ b/frontend/src/scenes/settings/organization/VerifiedDomains/verifiedDomainsLogic.ts @@ -102,9 +102,9 @@ export const verifiedDomainsLogic = kea([ return false }, verifyDomain: async () => { - const response = (await api.create( + const response = await api.create( `api/organizations/${values.currentOrganization?.id}/domains/${values.verifyModal}/verify` - )) as OrganizationDomainType + ) if (response.is_verified) { lemonToast.success('Domain verified successfully.') } else { @@ -158,12 +158,12 @@ export const verifiedDomainsLogic = kea([ if (!id) { return } - const response = (await api.update( + const response = await api.update( `api/organizations/${values.currentOrganization?.id}/domains/${payload.id}`, { ...updateParams, } - )) as OrganizationDomainType + ) breakpoint() actions.replaceDomain(response) actions.setConfigureSAMLModalId(null) diff --git a/frontend/src/scenes/settings/organization/inviteLogic.ts b/frontend/src/scenes/settings/organization/inviteLogic.ts index 878961f7332bf..3492c95ab2be1 100644 --- a/frontend/src/scenes/settings/organization/inviteLogic.ts +++ b/frontend/src/scenes/settings/organization/inviteLogic.ts @@ -1,7 +1,7 @@ import { actions, connect, events, kea, listeners, path, reducers, selectors } from 'kea' import { loaders } from 'kea-loaders' import { router, urlToAction } from 'kea-router' -import api from 'lib/api' +import api, { PaginatedResponse } from 'lib/api' import { OrganizationMembershipLevel } from 'lib/constants' import { lemonToast } from 'lib/lemon-ui/LemonToast/LemonToast' import { organizationLogic } from 'scenes/organizationLogic' @@ -49,7 +49,7 @@ export const inviteLogic = kea([ { inviteTeamMembers: async () => { if (!values.canSubmit) { - return { invites: [] } + return [] } const payload: Pick[] = @@ -57,7 +57,10 @@ export const inviteLogic = kea([ if (values.message) { payload.forEach((payload) => (payload.message = values.message)) } - return await api.create('api/organizations/@current/invites/bulk/', payload) + return await api.create( + 'api/organizations/@current/invites/bulk/', + payload + ) }, }, ], @@ -66,7 +69,11 @@ export const inviteLogic = kea([ { loadInvites: async () => { return organizationLogic.values.currentOrganization - ? (await api.get('api/organizations/@current/invites/')).results + ? ( + await api.get>( + 'api/organizations/@current/invites/' + ) + ).results : [] }, deleteInvite: async (invite: OrganizationInviteType) => { diff --git a/frontend/src/scenes/settings/types.ts b/frontend/src/scenes/settings/types.ts index 99c83027a9ba1..255c42ed847d4 100644 --- a/frontend/src/scenes/settings/types.ts +++ b/frontend/src/scenes/settings/types.ts @@ -32,7 +32,8 @@ export type SettingSectionId = | 'project-surveys' // TODO: This section is for backward compat – remove when Environments are rolled out | 'project-toolbar' // TODO: This section is for backward compat – remove when Environments are rolled out | 'project-integrations' // TODO: This section is for backward compat – remove when Environments are rolled out - | 'project-rbac' // TODO: This section is for backward compat – remove when Environments are rolled out + | 'project-access-control' // TODO: This section is for backward compat – remove when Environments are rolled out + | 'project-role-based-access-control' // TODO: This section is for backward compat – remove when Environments are rolled out | 'project-danger-zone' | 'organization-details' | 'organization-members' @@ -70,6 +71,8 @@ export type SettingId = | 'integration-slack' | 'integration-other' | 'integration-ip-allowlist' + | 'project-access-control' + | 'project-role-based-access-control' | 'environment-rbac' | 'environment-delete' | 'project-delete' diff --git a/frontend/src/scenes/settings/user/personalAPIKeysLogic.tsx b/frontend/src/scenes/settings/user/personalAPIKeysLogic.tsx index e4f78f929def5..be47278635896 100644 --- a/frontend/src/scenes/settings/user/personalAPIKeysLogic.tsx +++ b/frontend/src/scenes/settings/user/personalAPIKeysLogic.tsx @@ -9,7 +9,7 @@ import { lemonToast } from 'lib/lemon-ui/LemonToast/LemonToast' import { urls } from 'scenes/urls' import { userLogic } from 'scenes/userLogic' -import { OrganizationBasicType, PersonalAPIKeyType, TeamBasicType } from '~/types' +import { APIScopeObject, OrganizationBasicType, PersonalAPIKeyType, TeamBasicType } from '~/types' import type { personalAPIKeysLogicType } from './personalAPIKeysLogicType' @@ -32,7 +32,7 @@ export const API_KEY_SCOPE_PRESETS = [ ] export type APIScope = { - key: string + key: APIScopeObject info?: string | JSX.Element disabledActions?: ('read' | 'write')[] disabledWhenProjectScoped?: boolean diff --git a/frontend/src/scenes/teamActivityDescriber.tsx b/frontend/src/scenes/teamActivityDescriber.tsx index 7fd8e6cdfe289..faf12a40c37ec 100644 --- a/frontend/src/scenes/teamActivityDescriber.tsx +++ b/frontend/src/scenes/teamActivityDescriber.tsx @@ -335,6 +335,7 @@ const teamActionsMapping: Record< id: () => null, updated_at: () => null, uuid: () => null, + user_access_level: () => null, live_events_token: () => null, } diff --git a/frontend/src/scenes/teamLogic.tsx b/frontend/src/scenes/teamLogic.tsx index 2ebf076409061..532b271fa6638 100644 --- a/frontend/src/scenes/teamLogic.tsx +++ b/frontend/src/scenes/teamLogic.tsx @@ -182,7 +182,8 @@ export const teamLogic = kea([ (selectors) => [selectors.currentTeam, selectors.currentTeamLoading], // If project has been loaded and is still null, it means the user just doesn't have access. (currentTeam, currentTeamLoading): boolean => - !currentTeam?.effective_membership_level && !currentTeamLoading, + (!currentTeam?.effective_membership_level || currentTeam.user_access_level === 'none') && + !currentTeamLoading, ], demoOnlyProject: [ (selectors) => [selectors.currentTeam, organizationLogic.selectors.currentOrganization], @@ -204,8 +205,9 @@ export const teamLogic = kea([ isTeamTokenResetAvailable: [ (selectors) => [selectors.currentTeam], (currentTeam): boolean => - !!currentTeam?.effective_membership_level && - currentTeam.effective_membership_level >= OrganizationMembershipLevel.Admin, + (!!currentTeam?.effective_membership_level && + currentTeam.effective_membership_level >= OrganizationMembershipLevel.Admin) || + currentTeam?.user_access_level === 'admin', ], testAccountFilterFrequentMistakes: [ (selectors) => [selectors.currentTeam], diff --git a/frontend/src/scenes/userLogic.ts b/frontend/src/scenes/userLogic.ts index 9db1e96fa8806..a3adad1c6f50e 100644 --- a/frontend/src/scenes/userLogic.ts +++ b/frontend/src/scenes/userLogic.ts @@ -55,7 +55,7 @@ export const userLogic = kea([ { loadUser: async () => { try { - return await api.get('api/users/@me/') + return await api.get('api/users/@me/') } catch (error: any) { console.error(error) actions.loadUserFailure(error.message) @@ -67,12 +67,13 @@ export const userLogic = kea([ throw new Error('Current user has not been loaded yet, so it cannot be updated!') } try { - const response = await api.update('api/users/@me/', user) + const response = await api.update('api/users/@me/', user) successCallback && successCallback() return response } catch (error: any) { console.error(error) actions.updateUserFailure(error.message) + return values.user } }, setUserScenePersonalisation: async ({ scene, dashboard }) => { @@ -80,13 +81,14 @@ export const userLogic = kea([ throw new Error('Current user has not been loaded yet, so it cannot be updated!') } try { - return await api.create('api/users/@me/scene_personalisation', { + return await api.create('api/users/@me/scene_personalisation', { scene, dashboard, }) } catch (error: any) { console.error(error) actions.updateUserFailure(error.message) + return values.user } }, }, diff --git a/frontend/src/types.ts b/frontend/src/types.ts index e07f0597a2aac..417095ceaedd6 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -229,6 +229,10 @@ export interface ColumnConfig { active: ColumnChoice } +export type WithAccessControl = { + user_access_level: 'none' | 'member' | 'admin' | 'viewer' | 'editor' +} + interface UserBaseType { uuid: string distinct_id: string @@ -489,6 +493,7 @@ export interface ProjectType extends ProjectBasicType { export interface TeamSurveyConfigType { appearance?: SurveyAppearance } + export interface TeamType extends TeamBasicType { created_at: string updated_at: string @@ -2893,7 +2898,7 @@ export interface FeatureFlagBasicType { ensure_experience_continuity: boolean | null } -export interface FeatureFlagType extends Omit { +export interface FeatureFlagType extends Omit, WithAccessControl { /** Null means that the flag has never been saved yet (it's new). */ id: number | null created_by: UserBasicType | null @@ -3772,7 +3777,6 @@ export interface RoleType { name: string feature_flags_access_level: AccessLevel members: RoleMemberType[] - associated_flags: { id: number; key: string }[] created_at: string created_by: UserBasicType | null } @@ -3781,14 +3785,6 @@ export interface RolesListParams { feature_flags_access_level?: AccessLevel } -export interface FeatureFlagAssociatedRoleType { - id: string - feature_flag: FeatureFlagType | null - role: RoleType - updated_at: string - added_at: string -} - export interface RoleMemberType { id: string user: UserBaseType @@ -3798,6 +3794,80 @@ export interface RoleMemberType { user_uuid: string } +export type APIScopeObject = + | 'action' + | 'activity_log' + | 'annotation' + | 'batch_export' + | 'cohort' + | 'dashboard' + | 'dashboard_template' + | 'early_access_feature' + | 'event_definition' + | 'experiment' + | 'export' + | 'feature_flag' + | 'group' + | 'insight' + | 'query' + | 'notebook' + | 'organization' + | 'organization_member' + | 'person' + | 'plugin' + | 'project' + | 'property_definition' + | 'session_recording' + | 'session_recording_playlist' + | 'sharing_configuration' + | 'subscription' + | 'survey' + | 'user' + | 'webhook' + +export interface AccessControlTypeBase { + created_by: UserBasicType | null + created_at: string + updated_at: string + resource: APIScopeObject + access_level: string | null // TODO: Change to enum + organization_member?: OrganizationMemberType['id'] | null + role?: RoleType['id'] | null +} + +export interface AccessControlTypeProject extends AccessControlTypeBase {} + +export interface AccessControlTypeMember extends AccessControlTypeBase { + organization_member: OrganizationMemberType['id'] +} + +export interface AccessControlTypeRole extends AccessControlTypeBase { + role: RoleType['id'] +} + +export type AccessControlType = AccessControlTypeProject | AccessControlTypeMember | AccessControlTypeRole + +export type AccessControlUpdateType = Pick & { + resource?: AccessControlType['resource'] +} + +export type AccessControlResponseType = { + access_controls: AccessControlType[] + available_access_levels: string[] // TODO: Change to enum + user_access_level: string + default_access_level: string + user_can_edit_access_levels: boolean +} + +// TODO: To be deprecated +export interface FeatureFlagAssociatedRoleType { + id: string + feature_flag: FeatureFlagType | null + role: RoleType + updated_at: string + added_at: string +} +// TODO: To be deprecated export interface OrganizationResourcePermissionType { id: string resource: Resource @@ -3883,12 +3953,13 @@ export type NotebookListItemType = { last_modified_by?: UserBasicType | null } -export type NotebookType = NotebookListItemType & { - content: JSONContent | null - version: number - // used to power text-based search - text_content?: string | null -} +export type NotebookType = NotebookListItemType & + WithAccessControl & { + content: JSONContent | null + version: number + // used to power text-based search + text_content?: string | null + } export enum NotebookNodeType { Mention = 'ph-mention', @@ -4331,6 +4402,7 @@ export enum SidePanelTab { Discussion = 'discussion', Status = 'status', Exports = 'exports', + AccessControl = 'access-control', ExperimentFeatureFlag = 'experiment-feature-flag', } diff --git a/latest_migrations.manifest b/latest_migrations.manifest index f8f13e49ea0ad..33c08d0e56965 100644 --- a/latest_migrations.manifest +++ b/latest_migrations.manifest @@ -2,7 +2,7 @@ admin: 0003_logentry_add_action_flag_choices auth: 0012_alter_user_first_name_max_length axes: 0006_remove_accesslog_trusted contenttypes: 0002_remove_content_type_name -ee: 0016_rolemembership_organization_member +ee: 0017_accesscontrol otp_static: 0002_throttling otp_totp: 0002_auto_20190420_0723 posthog: 0494_team_project_non_null diff --git a/posthog/api/dashboards/dashboard.py b/posthog/api/dashboards/dashboard.py index ca626c0d1a8c2..008fec791884b 100644 --- a/posthog/api/dashboards/dashboard.py +++ b/posthog/api/dashboards/dashboard.py @@ -29,6 +29,8 @@ from posthog.models.dashboard_templates import DashboardTemplate from posthog.models.tagged_item import TaggedItem from posthog.models.user import User +from posthog.rbac.access_control_api_mixin import AccessControlViewSetMixin +from posthog.rbac.user_access_control import UserAccessControlSerializerMixin from posthog.user_permissions import UserPermissionsSerializerMixin from posthog.utils import filters_override_requested_by_client, variables_override_requested_by_client @@ -87,6 +89,7 @@ class DashboardBasicSerializer( TaggedItemSerializerMixin, serializers.ModelSerializer, UserPermissionsSerializerMixin, + UserAccessControlSerializerMixin, ): created_by = UserBasicSerializer(read_only=True) effective_privilege_level = serializers.SerializerMethodField() @@ -109,6 +112,7 @@ class Meta: "restriction_level", "effective_restriction_level", "effective_privilege_level", + "user_access_level", ] read_only_fields = fields @@ -157,8 +161,14 @@ class Meta: "restriction_level", "effective_restriction_level", "effective_privilege_level", + "user_access_level", + ] + read_only_fields = [ + "creation_mode", + "effective_restriction_level", + "is_shared", + "user_access_level", ] - read_only_fields = ["creation_mode", "effective_restriction_level", "is_shared"] def validate_filters(self, value) -> dict: if not isinstance(value, dict): @@ -449,10 +459,7 @@ def _update_creation_mode(self, validated_data, use_template: str, use_dashboard class DashboardsViewSet( - TeamAndOrgViewSetMixin, - TaggedItemViewSetMixin, - ForbidDestroyModel, - viewsets.ModelViewSet, + TeamAndOrgViewSetMixin, TaggedItemViewSetMixin, ForbidDestroyModel, AccessControlViewSetMixin, viewsets.ModelViewSet ): scope_object = "dashboard" queryset = Dashboard.objects_including_soft_deleted.order_by("-pinned", "name") diff --git a/posthog/api/documentation.py b/posthog/api/documentation.py index d06dd23a3151e..d885aa12a3c84 100644 --- a/posthog/api/documentation.py +++ b/posthog/api/documentation.py @@ -37,7 +37,7 @@ def get_security_requirement(self, auto_schema): for permission in auto_schema.view.get_permissions(): if isinstance(permission, APIScopePermission): try: - scopes = permission.get_required_scopes(request, view) + scopes = permission._get_required_scopes(request, view) return [{self.name: scopes}] except (PermissionDenied, ImproperlyConfigured): # NOTE: This should never happen - it indicates that we shouldn't be including it in the docs diff --git a/posthog/api/feature_flag.py b/posthog/api/feature_flag.py index 324eb87765441..1d01fae5b5654 100644 --- a/posthog/api/feature_flag.py +++ b/posthog/api/feature_flag.py @@ -57,6 +57,9 @@ from posthog.rate_limit import BurstRateThrottle from loginas.utils import is_impersonated_session +from posthog.rbac.access_control_api_mixin import AccessControlViewSetMixin +from posthog.rbac.user_access_control import UserAccessControlSerializerMixin + DATABASE_FOR_LOCAL_EVALUATION = ( "default" if ("local_evaluation" not in settings.READ_REPLICA_OPT_IN or "replica" not in settings.DATABASES) @@ -85,7 +88,9 @@ def has_object_permission(self, request: Request, view, feature_flag) -> bool: return can_user_edit_feature_flag(request, feature_flag) -class FeatureFlagSerializer(TaggedItemSerializerMixin, serializers.HyperlinkedModelSerializer): +class FeatureFlagSerializer( + TaggedItemSerializerMixin, UserAccessControlSerializerMixin, serializers.HyperlinkedModelSerializer +): created_by = UserBasicSerializer(read_only=True) # :TRICKY: Needed for backwards compatibility filters = serializers.DictField(source="get_filters", required=False) @@ -136,11 +141,16 @@ class Meta: "usage_dashboard", "analytics_dashboards", "has_enriched_analytics", + "user_access_level", ] def get_can_edit(self, feature_flag: FeatureFlag) -> bool: # TODO: make sure this isn't n+1 - return can_user_edit_feature_flag(self.context["request"], feature_flag) + + return ( + can_user_edit_feature_flag(self.context["request"], feature_flag) + and self.get_user_access_level(feature_flag) == "editor" + ) # Simple flags are ones that only have rollout_percentage #  That means server side libraries are able to gate these flags without calling to the server @@ -418,6 +428,7 @@ class Meta: class FeatureFlagViewSet( TeamAndOrgViewSetMixin, + AccessControlViewSetMixin, TaggedItemViewSetMixin, ForbidDestroyModel, viewsets.ModelViewSet, diff --git a/posthog/api/insight.py b/posthog/api/insight.py index d1aa643a400a0..2e34d7dc70fee 100644 --- a/posthog/api/insight.py +++ b/posthog/api/insight.py @@ -107,6 +107,8 @@ ClickHouseBurstRateThrottle, ClickHouseSustainedRateThrottle, ) +from posthog.rbac.access_control_api_mixin import AccessControlViewSetMixin +from posthog.rbac.user_access_control import UserAccessControlSerializerMixin from posthog.settings import CAPTURE_TIME_TO_SEE_DATA, SITE_URL from posthog.user_permissions import UserPermissionsSerializerMixin from posthog.utils import ( @@ -250,7 +252,7 @@ def _dashboard_tiles(self, instance): return [tile.dashboard_id for tile in instance.dashboard_tiles.all()] -class InsightSerializer(InsightBasicSerializer, UserPermissionsSerializerMixin): +class InsightSerializer(InsightBasicSerializer, UserPermissionsSerializerMixin, UserAccessControlSerializerMixin): result = serializers.SerializerMethodField() hasMore = serializers.SerializerMethodField() columns = serializers.SerializerMethodField() @@ -332,6 +334,7 @@ class Meta: "is_sample", "effective_restriction_level", "effective_privilege_level", + "user_access_level", "timezone", "is_cached", "query_status", @@ -348,6 +351,7 @@ class Meta: "is_sample", "effective_restriction_level", "effective_privilege_level", + "user_access_level", "timezone", "refreshing", "is_cached", @@ -710,6 +714,7 @@ def dashboard_tile_from_context(self, insight: Insight, dashboard: Optional[Dash ) class InsightViewSet( TeamAndOrgViewSetMixin, + AccessControlViewSetMixin, TaggedItemViewSetMixin, ForbidDestroyModel, viewsets.ModelViewSet, diff --git a/posthog/api/notebook.py b/posthog/api/notebook.py index 60dc30bde98e6..34ce1d304cd03 100644 --- a/posthog/api/notebook.py +++ b/posthog/api/notebook.py @@ -35,6 +35,8 @@ from posthog.models.activity_logging.activity_page import activity_page_response from posthog.models.notebook.notebook import Notebook from posthog.models.utils import UUIDT +from posthog.rbac.access_control_api_mixin import AccessControlViewSetMixin +from posthog.rbac.user_access_control import UserAccessControlSerializerMixin from posthog.utils import relative_date_parse from loginas.utils import is_impersonated_session @@ -95,7 +97,7 @@ class Meta: read_only_fields = fields -class NotebookSerializer(NotebookMinimalSerializer): +class NotebookSerializer(NotebookMinimalSerializer, UserAccessControlSerializerMixin): class Meta: model = Notebook fields = [ @@ -110,6 +112,7 @@ class Meta: "created_by", "last_modified_at", "last_modified_by", + "user_access_level", ] read_only_fields = [ "id", @@ -118,6 +121,7 @@ class Meta: "created_by", "last_modified_at", "last_modified_by", + "user_access_level", ] def create(self, validated_data: dict, *args, **kwargs) -> Notebook: @@ -235,7 +239,7 @@ def update(self, instance: Notebook, validated_data: dict, **kwargs) -> Notebook ], ) ) -class NotebookViewSet(TeamAndOrgViewSetMixin, ForbidDestroyModel, viewsets.ModelViewSet): +class NotebookViewSet(TeamAndOrgViewSetMixin, AccessControlViewSetMixin, ForbidDestroyModel, viewsets.ModelViewSet): scope_object = "notebook" queryset = Notebook.objects.all() filter_backends = [DjangoFilterBackend] diff --git a/posthog/api/organization.py b/posthog/api/organization.py index 3cdd6991a5134..a2cf38cb89828 100644 --- a/posthog/api/organization.py +++ b/posthog/api/organization.py @@ -3,7 +3,6 @@ from django.db.models import Model, QuerySet from django.shortcuts import get_object_or_404 -from django.views import View from rest_framework import exceptions, permissions, serializers, viewsets from rest_framework.request import Request @@ -20,12 +19,13 @@ from posthog.models.team.util import delete_bulky_postgres_data from posthog.models.uploaded_media import UploadedMedia from posthog.permissions import ( - CREATE_METHODS, + CREATE_ACTIONS, APIScopePermission, OrganizationAdminWritePermissions, TimeSensitiveActionPermission, extract_organization, ) +from posthog.rbac.user_access_control import UserAccessControlSerializerMixin from posthog.user_permissions import UserPermissions, UserPermissionsSerializerMixin @@ -39,7 +39,7 @@ def has_permission(self, request: Request, view) -> bool: if ( # Make multiple orgs only premium on self-hosted, since enforcement of this wouldn't make sense on Cloud not is_cloud() - and request.method in CREATE_METHODS + and view.action in CREATE_ACTIONS and ( user.organization is None or not user.organization.is_feature_available(AvailableFeature.ORGANIZATIONS_PROJECTS) @@ -51,7 +51,7 @@ def has_permission(self, request: Request, view) -> bool: class OrganizationPermissionsWithDelete(OrganizationAdminWritePermissions): - def has_object_permission(self, request: Request, view: View, object: Model) -> bool: + def has_object_permission(self, request: Request, view, object: Model) -> bool: if request.method in permissions.SAFE_METHODS: return True # TODO: Optimize so that this computation is only done once, on `OrganizationMemberPermissions` @@ -65,7 +65,9 @@ def has_object_permission(self, request: Request, view: View, object: Model) -> ) -class OrganizationSerializer(serializers.ModelSerializer, UserPermissionsSerializerMixin): +class OrganizationSerializer( + serializers.ModelSerializer, UserPermissionsSerializerMixin, UserAccessControlSerializerMixin +): membership_level = serializers.SerializerMethodField() teams = serializers.SerializerMethodField() projects = serializers.SerializerMethodField() @@ -126,7 +128,15 @@ def get_membership_level(self, organization: Organization) -> Optional[Organizat return membership.level if membership is not None else None def get_teams(self, instance: Organization) -> list[dict[str, Any]]: - visible_teams = instance.teams.filter(id__in=self.user_permissions.team_ids_visible_for_user) + # Support new access control system + + visible_teams = ( + self.user_access_control.filter_queryset_by_access_level(instance.teams, include_all_if_admin=True) + if self.user_access_control + else instance.teams.none() + ) + # Support old access control system + visible_teams = visible_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_projects(self, instance: Organization) -> list[dict[str, Any]]: diff --git a/posthog/api/organization_member.py b/posthog/api/organization_member.py index aaf1d5b802776..f176969ace2e3 100644 --- a/posthog/api/organization_member.py +++ b/posthog/api/organization_member.py @@ -2,7 +2,6 @@ from django.db.models import Model, Prefetch, QuerySet, F from django.shortcuts import get_object_or_404 -from django.views import View from django_otp.plugins.otp_totp.models import TOTPDevice from rest_framework import exceptions, mixins, serializers, viewsets from rest_framework.permissions import SAFE_METHODS, BasePermission @@ -23,7 +22,7 @@ class OrganizationMemberObjectPermissions(BasePermission): message = "Your cannot edit other organization members." - def has_object_permission(self, request: Request, view: View, membership: OrganizationMembership) -> bool: + def has_object_permission(self, request: Request, view, membership: OrganizationMembership) -> bool: if request.method in SAFE_METHODS: return True organization = extract_organization(membership, view) diff --git a/posthog/api/personal_api_key.py b/posthog/api/personal_api_key.py index 23f6d531693a0..355d6cc1c189a 100644 --- a/posthog/api/personal_api_key.py +++ b/posthog/api/personal_api_key.py @@ -5,7 +5,8 @@ from rest_framework.permissions import IsAuthenticated from posthog.models import PersonalAPIKey, User -from posthog.models.personal_api_key import API_SCOPE_ACTIONS, API_SCOPE_OBJECTS, hash_key_value, mask_key_value +from posthog.models.personal_api_key import hash_key_value, mask_key_value +from posthog.models.scopes import API_SCOPE_ACTIONS, API_SCOPE_OBJECTS from posthog.models.team.team import Team from posthog.models.utils import generate_random_token_personal from posthog.permissions import TimeSensitiveActionPermission diff --git a/posthog/api/project.py b/posthog/api/project.py index 6fbd4e42891f3..1093f26d8dc52 100644 --- a/posthog/api/project.py +++ b/posthog/api/project.py @@ -8,6 +8,8 @@ from rest_framework.decorators import action from rest_framework.permissions import IsAuthenticated +from ee.api.rbac.access_control import AccessControlViewSetMixin +from posthog.geoip import get_geoip_properties from posthog.api.routing import TeamAndOrgViewSetMixin from posthog.api.shared import ProjectBackwardCompatBasicSerializer from posthog.api.team import ( @@ -16,7 +18,6 @@ validate_team_attrs, ) from posthog.event_usage import report_user_action -from posthog.geoip import get_geoip_properties from posthog.jwt import PosthogJwtAudience, encode_jwt from posthog.models import User from posthog.models.activity_logging.activity_log import ( @@ -29,7 +30,7 @@ from posthog.models.async_deletion import AsyncDeletion, DeletionType from posthog.models.group_type_mapping import GroupTypeMapping from posthog.models.organization import OrganizationMembership -from posthog.models.personal_api_key import APIScopeObjectOrNotSupported +from posthog.models.scopes import APIScopeObjectOrNotSupported from posthog.models.product_intent.product_intent import ProductIntent from posthog.models.project import Project from posthog.models.signals import mute_selected_signals @@ -383,7 +384,7 @@ def update(self, instance: Project, validated_data: dict[str, Any]) -> Project: return instance -class ProjectViewSet(TeamAndOrgViewSetMixin, viewsets.ModelViewSet): +class ProjectViewSet(TeamAndOrgViewSetMixin, AccessControlViewSetMixin, viewsets.ModelViewSet): """ Projects for the current organization. """ diff --git a/posthog/api/routing.py b/posthog/api/routing.py index 9ff2fede76449..797d1817e6650 100644 --- a/posthog/api/routing.py +++ b/posthog/api/routing.py @@ -18,16 +18,18 @@ SharingAccessTokenAuthentication, ) from posthog.models.organization import Organization -from posthog.models.personal_api_key import APIScopeObjectOrNotSupported +from posthog.models.scopes import APIScopeObjectOrNotSupported from posthog.models.project import Project from posthog.models.team import Team from posthog.models.user import User from posthog.permissions import ( APIScopePermission, + AccessControlPermission, OrganizationMemberPermissions, SharingTokenPermission, TeamMemberAccessPermission, ) +from posthog.rbac.user_access_control import UserAccessControl from posthog.user_permissions import UserPermissions if TYPE_CHECKING: @@ -59,7 +61,6 @@ class TeamAndOrgViewSetMixin(_GenericViewSet): # TODO: Rename to include "Env" authentication_classes = [] permission_classes = [] - # NOTE: Could we type this? Would be pretty cool as a helper scope_object: Optional[APIScopeObjectOrNotSupported] = None required_scopes: Optional[list[str]] = None sharing_enabled_actions: list[str] = [] @@ -101,7 +102,7 @@ def get_permissions(self): # NOTE: We define these here to make it hard _not_ to use them. If you want to override them, you have to # override the entire method. - permission_classes: list = [IsAuthenticated, APIScopePermission] + permission_classes: list = [IsAuthenticated, APIScopePermission, AccessControlPermission] if self._is_team_view or self._is_project_view: permission_classes.append(TeamMemberAccessPermission) @@ -157,7 +158,25 @@ def get_queryset(self) -> QuerySet: except NotImplementedError: pass - return self._filter_queryset_by_parents_lookups(queryset) + queryset = self._filter_queryset_by_parents_lookups(queryset) + + 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 + + # NOTE: Half implemented - for admins, they may want to include listing of results that are not accessible (like private resources) + include_all_if_admin = self.request.GET.get("admin_include_all") == "true" + + # Additionally "projects" is a special one where we always want to include all projects if you're an org admin + if self.scope_object == "project": + include_all_if_admin = True + + queryset = self.user_access_control.filter_queryset_by_access_level( + queryset, include_all_if_admin=include_all_if_admin + ) + + return queryset def dangerously_get_object(self) -> Any: """ @@ -408,3 +427,13 @@ def _get_team_from_request(self) -> Optional["Team"]: @cached_property def user_permissions(self) -> "UserPermissions": return UserPermissions(user=cast(User, self.request.user), team=self.team) + + @cached_property + def user_access_control(self) -> "UserAccessControl": + team: Optional[Team] = None + try: + team = self.team + except (Team.DoesNotExist, KeyError): + pass + + return UserAccessControl(user=cast(User, self.request.user), team=team, organization_id=self.organization_id) diff --git a/posthog/api/search.py b/posthog/api/search.py index cb5a779b0ed1f..c5f1e94f3f311 100644 --- a/posthog/api/search.py +++ b/posthog/api/search.py @@ -100,6 +100,7 @@ def list(self, request: Request, **kw) -> HttpResponse: # add entities for entity_meta in [ENTITY_MAP[entity] for entity in entities]: klass_qs, entity_name = class_queryset( + view=self, klass=entity_meta.get("klass"), project_id=self.project_id, query=query, @@ -134,6 +135,7 @@ def process_query(query: str): def class_queryset( + view: TeamAndOrgViewSetMixin, klass: type[Model], project_id: int, query: str | None, @@ -145,6 +147,8 @@ def class_queryset( values = ["type", "result_id", "extra_fields"] qs: QuerySet[Any] = klass.objects.filter(team__project_id=project_id) # filter team + qs = view.user_access_control.filter_queryset_by_access_level(qs) # filter access level + # :TRICKY: can't use an annotation here as `type` conflicts with a field on some models qs = qs.extra(select={"type": f"'{entity_type}'"}) # entity type diff --git a/posthog/api/team.py b/posthog/api/team.py index 566ec7fad57ed..7ba1bb2afbf3d 100644 --- a/posthog/api/team.py +++ b/posthog/api/team.py @@ -27,20 +27,23 @@ from posthog.models.async_deletion import AsyncDeletion, DeletionType from posthog.models.group_type_mapping import GroupTypeMapping from posthog.models.organization import OrganizationMembership -from posthog.models.personal_api_key import APIScopeObjectOrNotSupported +from posthog.models.scopes import APIScopeObjectOrNotSupported from posthog.models.project import Project from posthog.models.signals import mute_selected_signals from posthog.models.team.util import delete_batch_exports, delete_bulky_postgres_data from posthog.models.utils import UUIDT from posthog.permissions import ( - CREATE_METHODS, + CREATE_ACTIONS, APIScopePermission, + AccessControlPermission, OrganizationAdminWritePermissions, OrganizationMemberPermissions, TeamMemberLightManagementPermission, TeamMemberStrictManagementPermission, get_organization_from_view, ) +from posthog.rbac.access_control_api_mixin import AccessControlViewSetMixin +from posthog.rbac.user_access_control import UserAccessControlSerializerMixin from posthog.user_permissions import UserPermissions, UserPermissionsSerializerMixin from posthog.utils import get_ip_address, get_week_start_for_country_code @@ -51,7 +54,7 @@ class PremiumMultiProjectPermissions(BasePermission): # TODO: Rename to include message = "You must upgrade your PostHog plan to be able to create and manage multiple projects or environments." def has_permission(self, request: request.Request, view) -> bool: - if request.method in CREATE_METHODS: + if view.action in CREATE_ACTIONS: try: organization = get_organization_from_view(view) except ValueError: @@ -113,7 +116,7 @@ class Meta: ] -class TeamSerializer(serializers.ModelSerializer, UserPermissionsSerializerMixin): +class TeamSerializer(serializers.ModelSerializer, UserPermissionsSerializerMixin, UserAccessControlSerializerMixin): instance: Optional[Team] effective_membership_level = serializers.SerializerMethodField() @@ -174,6 +177,7 @@ class Meta: "default_modifiers", "has_completed_onboarding_for", "surveys_opt_in", + "user_access_level", "heatmaps_opt_in", "live_events_token", "product_intents", @@ -191,10 +195,12 @@ class Meta: "has_group_types", "default_modifiers", "person_on_events_querying_enabled", + "user_access_level", "live_events_token", ) def get_effective_membership_level(self, team: Team) -> Optional[OrganizationMembership.Level]: + # TODO: Map from user_access_controls return self.user_permissions.team(team).effective_membership_level def get_has_group_types(self, team: Team) -> bool: @@ -411,7 +417,7 @@ def update(self, instance: Team, validated_data: dict[str, Any]) -> Team: return updated_team -class TeamViewSet(TeamAndOrgViewSetMixin, viewsets.ModelViewSet): +class TeamViewSet(TeamAndOrgViewSetMixin, AccessControlViewSetMixin, viewsets.ModelViewSet): """ Projects for the current organization. """ @@ -441,6 +447,7 @@ def dangerously_get_permissions(self) -> list: permissions: list = [ IsAuthenticated, APIScopePermission, + AccessControlPermission, PremiumMultiProjectPermissions, *self.permission_classes, ] diff --git a/posthog/api/test/__snapshots__/test_action.ambr b/posthog/api/test/__snapshots__/test_action.ambr index a55b9fe7296a2..bbc770428838f 100644 --- a/posthog/api/test/__snapshots__/test_action.ambr +++ b/posthog/api/test/__snapshots__/test_action.ambr @@ -329,7 +329,7 @@ WHERE "posthog_organizationmembership"."user_id" = 2 ''' # --- -# name: TestActionApi.test_listing_actions_is_not_nplus1.3 +# name: TestActionApi.test_listing_actions_is_not_nplus1.5 ''' SELECT "posthog_organization"."id", "posthog_organization"."name", diff --git a/posthog/api/test/__snapshots__/test_annotation.ambr b/posthog/api/test/__snapshots__/test_annotation.ambr index 6f4850c626049..e554c57e452a5 100644 --- a/posthog/api/test/__snapshots__/test_annotation.ambr +++ b/posthog/api/test/__snapshots__/test_annotation.ambr @@ -272,7 +272,53 @@ LIMIT 1000 ''' # --- -# name: TestAnnotation.test_retrieving_annotation_is_not_n_plus_1.2 +# name: TestAnnotation.test_retrieving_annotation_is_not_n_plus_1.3 + ''' + SELECT "ee_accesscontrol"."id", + "ee_accesscontrol"."team_id", + "ee_accesscontrol"."resource", + "ee_accesscontrol"."access_level", + "ee_accesscontrol"."resource_id", + "ee_accesscontrol"."organization_member_id", + "ee_accesscontrol"."role_id", + "ee_accesscontrol"."created_by_id", + "ee_accesscontrol"."created_at", + "ee_accesscontrol"."updated_at" + FROM "ee_accesscontrol" + LEFT OUTER JOIN "posthog_organizationmembership" ON ("ee_accesscontrol"."organization_member_id" = "posthog_organizationmembership"."id") + WHERE (("ee_accesscontrol"."organization_member_id" IS NULL + AND "ee_accesscontrol"."resource" = 'project' + AND "ee_accesscontrol"."resource_id" = '227' + AND "ee_accesscontrol"."role_id" IS NULL + AND "ee_accesscontrol"."team_id" = 2) + OR ("posthog_organizationmembership"."user_id" = 2 + AND "ee_accesscontrol"."resource" = 'project' + AND "ee_accesscontrol"."resource_id" = '227' + AND "ee_accesscontrol"."role_id" IS NULL + AND "ee_accesscontrol"."team_id" = 2) + OR ("ee_accesscontrol"."organization_member_id" IS NULL + AND "ee_accesscontrol"."resource" = 'annotation' + AND "ee_accesscontrol"."resource_id" IS NULL + AND "ee_accesscontrol"."role_id" IS NULL + AND "ee_accesscontrol"."team_id" = 2) + OR ("posthog_organizationmembership"."user_id" = 2 + AND "ee_accesscontrol"."resource" = 'annotation' + AND "ee_accesscontrol"."resource_id" IS NULL + AND "ee_accesscontrol"."role_id" IS NULL + AND "ee_accesscontrol"."team_id" = 2) + OR ("ee_accesscontrol"."organization_member_id" IS NULL + AND "ee_accesscontrol"."resource" = 'annotation' + AND "ee_accesscontrol"."resource_id" IS NOT NULL + AND "ee_accesscontrol"."role_id" IS NULL + AND "ee_accesscontrol"."team_id" = 2) + OR ("posthog_organizationmembership"."user_id" = 2 + AND "ee_accesscontrol"."resource" = 'annotation' + AND "ee_accesscontrol"."resource_id" IS NOT NULL + AND "ee_accesscontrol"."role_id" IS NULL + AND "ee_accesscontrol"."team_id" = 2)) + ''' +# --- +# name: TestAnnotation.test_retrieving_annotation_is_not_n_plus_1.4 ''' SELECT "posthog_organizationmembership"."id", "posthog_organizationmembership"."organization_id", @@ -304,7 +350,7 @@ WHERE "posthog_organizationmembership"."user_id" = 2 ''' # --- -# name: TestAnnotation.test_retrieving_annotation_is_not_n_plus_1.3 +# name: TestAnnotation.test_retrieving_annotation_is_not_n_plus_1.5 ''' SELECT COUNT(*) AS "__count" FROM "posthog_annotation" diff --git a/posthog/api/test/__snapshots__/test_feature_flag.ambr b/posthog/api/test/__snapshots__/test_feature_flag.ambr index 0c17549699ba7..f9f95b698b9a3 100644 --- a/posthog/api/test/__snapshots__/test_feature_flag.ambr +++ b/posthog/api/test/__snapshots__/test_feature_flag.ambr @@ -29,312 +29,6 @@ HAVING max(is_deleted) = 0 SETTINGS optimize_aggregation_in_order = 1) ''' # --- -# name: TestBlastRadius.test_user_blast_radius_with_groups - ''' - /* user_id:0 request:_snapshot_ */ - SELECT count(1) - FROM - (SELECT group_key, - argMax(group_properties, _timestamp) AS group_properties_0 - FROM groups - WHERE team_id = 2 - AND group_type_index = 0 - GROUP BY group_key - HAVING 1=1 - AND (has(['0', '1', '2', '3'], replaceRegexpAll(JSONExtractRaw(group_properties_0, 'industry'), '^"|"$', '')))) - ''' -# --- -# name: TestBlastRadius.test_user_blast_radius_with_groups.1 - ''' - /* user_id:0 request:_snapshot_ */ - SELECT count(DISTINCT group_key) - FROM groups - WHERE team_id = 2 - AND group_type_index = 0 - ''' -# --- -# name: TestBlastRadius.test_user_blast_radius_with_groups_multiple_queries - ''' - /* user_id:0 request:_snapshot_ */ - SELECT count(1) - FROM - (SELECT group_key, - argMax(group_properties, _timestamp) AS group_properties_0 - FROM groups - WHERE team_id = 2 - AND group_type_index = 0 - GROUP BY group_key - HAVING 1=1 - AND ((has(['0', '1', '2', '3', '4'], replaceRegexpAll(JSONExtractRaw(group_properties_0, 'industry'), '^"|"$', ''))) - AND (has(['2', '3', '4', '5', '6'], replaceRegexpAll(JSONExtractRaw(group_properties_0, 'industry'), '^"|"$', ''))))) - ''' -# --- -# name: TestBlastRadius.test_user_blast_radius_with_groups_multiple_queries.1 - ''' - /* user_id:0 request:_snapshot_ */ - SELECT count(DISTINCT group_key) - FROM groups - WHERE team_id = 2 - AND group_type_index = 0 - ''' -# --- -# name: TestBlastRadius.test_user_blast_radius_with_multiple_precalculated_cohorts - ''' - - SELECT count(DISTINCT person_id) - FROM cohortpeople - WHERE team_id = 2 - AND cohort_id = 2 - AND version = NULL - ''' -# --- -# name: TestBlastRadius.test_user_blast_radius_with_multiple_precalculated_cohorts.1 - ''' - /* cohort_calculation: */ - SELECT count(DISTINCT person_id) - FROM cohortpeople - WHERE team_id = 2 - AND cohort_id = 2 - AND version = 0 - ''' -# --- -# name: TestBlastRadius.test_user_blast_radius_with_multiple_precalculated_cohorts.2 - ''' - - SELECT count(DISTINCT person_id) - FROM cohortpeople - WHERE team_id = 2 - AND cohort_id = 2 - AND version = NULL - ''' -# --- -# name: TestBlastRadius.test_user_blast_radius_with_multiple_precalculated_cohorts.3 - ''' - /* cohort_calculation: */ - SELECT count(DISTINCT person_id) - FROM cohortpeople - WHERE team_id = 2 - AND cohort_id = 2 - AND version = 0 - ''' -# --- -# name: TestBlastRadius.test_user_blast_radius_with_multiple_precalculated_cohorts.4 - ''' - /* user_id:0 request:_snapshot_ */ - SELECT count(1) - FROM - (SELECT id - FROM person - WHERE team_id = 2 - AND id in - (SELECT DISTINCT person_id - FROM cohortpeople - WHERE team_id = 2 - AND cohort_id = 2 - AND version = 0 ) - AND id in - (SELECT DISTINCT person_id - FROM cohortpeople - WHERE team_id = 2 - AND cohort_id = 2 - AND version = 0 ) - GROUP BY id - HAVING max(is_deleted) = 0 SETTINGS optimize_aggregation_in_order = 1) - ''' -# --- -# name: TestBlastRadius.test_user_blast_radius_with_multiple_precalculated_cohorts.5 - ''' - /* user_id:0 request:_snapshot_ */ - SELECT count(1) - FROM - (SELECT id - FROM person - WHERE team_id = 2 - GROUP BY id - HAVING max(is_deleted) = 0 SETTINGS optimize_aggregation_in_order = 1) - ''' -# --- -# name: TestBlastRadius.test_user_blast_radius_with_multiple_static_cohorts - ''' - - SELECT count(DISTINCT person_id) - FROM person_static_cohort - WHERE team_id = 2 - AND cohort_id = 2 - ''' -# --- -# name: TestBlastRadius.test_user_blast_radius_with_multiple_static_cohorts.1 - ''' - /* user_id:0 request:_snapshot_ */ - SELECT count(1) - FROM - (SELECT id - FROM person - WHERE team_id = 2 - AND id IN - (SELECT id - FROM person - INNER JOIN - (SELECT person_id - FROM person_static_cohort - WHERE team_id = 2 - AND cohort_id = 2 - GROUP BY person_id, - cohort_id, - team_id) cohort_persons ON cohort_persons.person_id = person.id - WHERE team_id = 2 - AND ((has(['1', '2', '4', '5', '6'], replaceRegexpAll(JSONExtractRaw(properties, 'group'), '^"|"$', '')))) ) - GROUP BY id - HAVING max(is_deleted) = 0 - AND ((has(['1', '2', '4', '5', '6'], replaceRegexpAll(JSONExtractRaw(argMax(person.properties, version), 'group'), '^"|"$', '')))) SETTINGS optimize_aggregation_in_order = 1) - ''' -# --- -# name: TestBlastRadius.test_user_blast_radius_with_multiple_static_cohorts.2 - ''' - /* user_id:0 request:_snapshot_ */ - SELECT count(1) - FROM - (SELECT id - FROM person - WHERE team_id = 2 - GROUP BY id - HAVING max(is_deleted) = 0 SETTINGS optimize_aggregation_in_order = 1) - ''' -# --- -# name: TestBlastRadius.test_user_blast_radius_with_multiple_static_cohorts.3 - ''' - - SELECT count(DISTINCT person_id) - FROM cohortpeople - WHERE team_id = 2 - AND cohort_id = 2 - AND version = NULL - ''' -# --- -# name: TestBlastRadius.test_user_blast_radius_with_multiple_static_cohorts.4 - ''' - /* cohort_calculation: */ - SELECT count(DISTINCT person_id) - FROM cohortpeople - WHERE team_id = 2 - AND cohort_id = 2 - AND version = 0 - ''' -# --- -# name: TestBlastRadius.test_user_blast_radius_with_multiple_static_cohorts.5 - ''' - - SELECT count(DISTINCT person_id) - FROM cohortpeople - WHERE team_id = 2 - AND cohort_id = 2 - AND version = NULL - ''' -# --- -# name: TestBlastRadius.test_user_blast_radius_with_multiple_static_cohorts.6 - ''' - /* cohort_calculation: */ - SELECT count(DISTINCT person_id) - FROM cohortpeople - WHERE team_id = 2 - AND cohort_id = 2 - AND version = 0 - ''' -# --- -# name: TestBlastRadius.test_user_blast_radius_with_multiple_static_cohorts.7 - ''' - /* user_id:0 request:_snapshot_ */ - SELECT count(1) - FROM - (SELECT id - FROM person - WHERE team_id = 2 - AND id in - (SELECT person_id as id - FROM person_static_cohort - WHERE cohort_id = 2 - AND team_id = 2) - AND id in - (SELECT DISTINCT person_id - FROM cohortpeople - WHERE team_id = 2 - AND cohort_id = 2 - AND version = 0 ) - GROUP BY id - HAVING max(is_deleted) = 0 SETTINGS optimize_aggregation_in_order = 1) - ''' -# --- -# name: TestBlastRadius.test_user_blast_radius_with_single_cohort - ''' - /* user_id:0 request:_snapshot_ */ - SELECT count(1) - FROM - (SELECT id - FROM person - WHERE team_id = 2 - AND id IN - (SELECT id - FROM person - WHERE team_id = 2 - AND (((has(['none'], replaceRegexpAll(JSONExtractRaw(properties, 'group'), '^"|"$', ''))) - OR (has(['1', '2', '3'], replaceRegexpAll(JSONExtractRaw(properties, 'group'), '^"|"$', ''))))) ) - GROUP BY id - HAVING max(is_deleted) = 0 - AND (((has(['none'], replaceRegexpAll(JSONExtractRaw(argMax(person.properties, version), 'group'), '^"|"$', ''))) - OR (has(['1', '2', '3'], replaceRegexpAll(JSONExtractRaw(argMax(person.properties, version), 'group'), '^"|"$', ''))))) SETTINGS optimize_aggregation_in_order = 1) - ''' -# --- -# name: TestBlastRadius.test_user_blast_radius_with_single_cohort.1 - ''' - /* user_id:0 request:_snapshot_ */ - SELECT count(1) - FROM - (SELECT id - FROM person - WHERE team_id = 2 - GROUP BY id - HAVING max(is_deleted) = 0 SETTINGS optimize_aggregation_in_order = 1) - ''' -# --- -# name: TestBlastRadius.test_user_blast_radius_with_single_cohort.2 - ''' - - SELECT count(DISTINCT person_id) - FROM cohortpeople - WHERE team_id = 2 - AND cohort_id = 2 - AND version = NULL - ''' -# --- -# name: TestBlastRadius.test_user_blast_radius_with_single_cohort.3 - ''' - /* cohort_calculation: */ - SELECT count(DISTINCT person_id) - FROM cohortpeople - WHERE team_id = 2 - AND cohort_id = 2 - AND version = 0 - ''' -# --- -# name: TestBlastRadius.test_user_blast_radius_with_single_cohort.4 - ''' - /* user_id:0 request:_snapshot_ */ - SELECT count(1) - FROM - (SELECT id - FROM person - INNER JOIN - (SELECT DISTINCT person_id - FROM cohortpeople - WHERE team_id = 2 - AND cohort_id = 2 - AND version = 0 - ORDER BY person_id) cohort_persons ON cohort_persons.person_id = person.id - WHERE team_id = 2 - GROUP BY id - HAVING max(is_deleted) = 0 SETTINGS optimize_aggregation_in_order = 1) - ''' -# --- # name: TestCohortGenerationForFeatureFlag.test_creating_static_cohort_iterator ''' SELECT "posthog_featureflag"."id", @@ -1871,7 +1565,9 @@ "posthog_organization"."domain_whitelist" FROM "posthog_organizationmembership" INNER JOIN "posthog_organization" ON ("posthog_organizationmembership"."organization_id" = "posthog_organization"."id") - WHERE "posthog_organizationmembership"."user_id" = 2 + WHERE ("posthog_organizationmembership"."organization_id" = '00000000-0000-0000-0000-000000000000'::uuid + AND "posthog_organizationmembership"."user_id" = 2) + LIMIT 21 ''' # --- # name: TestFeatureFlag.test_creating_static_cohort.3 diff --git a/posthog/api/test/__snapshots__/test_insight.ambr b/posthog/api/test/__snapshots__/test_insight.ambr index f1d09748e9b6c..81a733d4c6380 100644 --- a/posthog/api/test/__snapshots__/test_insight.ambr +++ b/posthog/api/test/__snapshots__/test_insight.ambr @@ -781,6 +781,13 @@ "posthog_team"."modifiers", "posthog_team"."correlation_config", "posthog_team"."session_recording_retention_period_days", + "posthog_team"."plugins_opt_in", + "posthog_team"."opt_out_capture", + "posthog_team"."event_names", + "posthog_team"."event_names_with_usage", + "posthog_team"."event_properties", + "posthog_team"."event_properties_with_usage", + "posthog_team"."event_properties_numerical", "posthog_team"."external_data_workspace_id", "posthog_team"."external_data_workspace_last_synced_at" FROM "posthog_team" @@ -789,6 +796,119 @@ ''' # --- # name: TestInsight.test_listing_insights_does_not_nplus1.11 + ''' + SELECT "posthog_team"."id", + "posthog_team"."uuid", + "posthog_team"."organization_id", + "posthog_team"."project_id", + "posthog_team"."api_token", + "posthog_team"."app_urls", + "posthog_team"."name", + "posthog_team"."slack_incoming_webhook", + "posthog_team"."created_at", + "posthog_team"."updated_at", + "posthog_team"."anonymize_ips", + "posthog_team"."completed_snippet_onboarding", + "posthog_team"."has_completed_onboarding_for", + "posthog_team"."ingested_event", + "posthog_team"."autocapture_opt_out", + "posthog_team"."autocapture_exceptions_opt_in", + "posthog_team"."autocapture_exceptions_errors_to_ignore", + "posthog_team"."session_recording_opt_in", + "posthog_team"."session_recording_sample_rate", + "posthog_team"."session_recording_minimum_duration_milliseconds", + "posthog_team"."session_recording_linked_flag", + "posthog_team"."session_recording_network_payload_capture_config", + "posthog_team"."session_replay_config", + "posthog_team"."capture_console_log_opt_in", + "posthog_team"."capture_performance_opt_in", + "posthog_team"."surveys_opt_in", + "posthog_team"."session_recording_version", + "posthog_team"."signup_token", + "posthog_team"."is_demo", + "posthog_team"."access_control", + "posthog_team"."week_start_day", + "posthog_team"."inject_web_apps", + "posthog_team"."test_account_filters", + "posthog_team"."test_account_filters_default_checked", + "posthog_team"."path_cleaning_filters", + "posthog_team"."timezone", + "posthog_team"."data_attributes", + "posthog_team"."person_display_name_properties", + "posthog_team"."live_events_columns", + "posthog_team"."recording_domains", + "posthog_team"."primary_dashboard_id", + "posthog_team"."extra_settings", + "posthog_team"."correlation_config", + "posthog_team"."session_recording_retention_period_days", + "posthog_team"."plugins_opt_in", + "posthog_team"."opt_out_capture", + "posthog_team"."event_names", + "posthog_team"."event_names_with_usage", + "posthog_team"."event_properties", + "posthog_team"."event_properties_with_usage", + "posthog_team"."event_properties_numerical", + "posthog_team"."external_data_workspace_id", + "posthog_team"."external_data_workspace_last_synced_at" + FROM "posthog_team" + WHERE "posthog_team"."id" = 2 + LIMIT 21 + ''' +# --- +# name: TestInsight.test_listing_insights_does_not_nplus1.12 + ''' + SELECT "posthog_team"."id", + "posthog_team"."uuid", + "posthog_team"."organization_id", + "posthog_team"."project_id", + "posthog_team"."api_token", + "posthog_team"."app_urls", + "posthog_team"."name", + "posthog_team"."slack_incoming_webhook", + "posthog_team"."created_at", + "posthog_team"."updated_at", + "posthog_team"."anonymize_ips", + "posthog_team"."completed_snippet_onboarding", + "posthog_team"."has_completed_onboarding_for", + "posthog_team"."ingested_event", + "posthog_team"."autocapture_opt_out", + "posthog_team"."autocapture_exceptions_opt_in", + "posthog_team"."autocapture_exceptions_errors_to_ignore", + "posthog_team"."session_recording_opt_in", + "posthog_team"."session_recording_sample_rate", + "posthog_team"."session_recording_minimum_duration_milliseconds", + "posthog_team"."session_recording_linked_flag", + "posthog_team"."session_recording_network_payload_capture_config", + "posthog_team"."session_replay_config", + "posthog_team"."capture_console_log_opt_in", + "posthog_team"."capture_performance_opt_in", + "posthog_team"."surveys_opt_in", + "posthog_team"."session_recording_version", + "posthog_team"."signup_token", + "posthog_team"."is_demo", + "posthog_team"."access_control", + "posthog_team"."week_start_day", + "posthog_team"."inject_web_apps", + "posthog_team"."test_account_filters", + "posthog_team"."test_account_filters_default_checked", + "posthog_team"."path_cleaning_filters", + "posthog_team"."timezone", + "posthog_team"."data_attributes", + "posthog_team"."person_display_name_properties", + "posthog_team"."live_events_columns", + "posthog_team"."recording_domains", + "posthog_team"."primary_dashboard_id", + "posthog_team"."extra_settings", + "posthog_team"."correlation_config", + "posthog_team"."session_recording_retention_period_days", + "posthog_team"."external_data_workspace_id", + "posthog_team"."external_data_workspace_last_synced_at" + FROM "posthog_team" + WHERE "posthog_team"."id" = 2 + LIMIT 21 + ''' +# --- +# name: TestInsight.test_listing_insights_does_not_nplus1.13 ''' SELECT "posthog_dashboardtile"."id", "posthog_dashboardtile"."dashboard_id", @@ -806,7 +926,7 @@ LIMIT 21 ''' # --- -# name: TestInsight.test_listing_insights_does_not_nplus1.12 +# name: TestInsight.test_listing_insights_does_not_nplus1.14 ''' SELECT "posthog_dashboarditem"."id", "posthog_dashboarditem"."name", @@ -841,7 +961,7 @@ LIMIT 21 ''' # --- -# name: TestInsight.test_listing_insights_does_not_nplus1.13 +# name: TestInsight.test_listing_insights_does_not_nplus1.15 ''' SELECT "posthog_dashboard"."id", "posthog_dashboard"."name", @@ -865,7 +985,7 @@ LIMIT 21 ''' # --- -# name: TestInsight.test_listing_insights_does_not_nplus1.14 +# name: TestInsight.test_listing_insights_does_not_nplus1.16 ''' SELECT "posthog_team"."id", "posthog_team"."uuid", @@ -931,7 +1051,7 @@ LIMIT 21 ''' # --- -# name: TestInsight.test_listing_insights_does_not_nplus1.15 +# name: TestInsight.test_listing_insights_does_not_nplus1.17 ''' SELECT "posthog_organization"."id", "posthog_organization"."name", @@ -957,7 +1077,7 @@ LIMIT 21 ''' # --- -# name: TestInsight.test_listing_insights_does_not_nplus1.16 +# name: TestInsight.test_listing_insights_does_not_nplus1.18 ''' SELECT "posthog_dashboard"."id", "posthog_dashboard"."name", @@ -982,7 +1102,7 @@ AND "posthog_dashboardtile"."insight_id" = 2) ''' # --- -# name: TestInsight.test_listing_insights_does_not_nplus1.17 +# name: TestInsight.test_listing_insights_does_not_nplus1.19 ''' SELECT "posthog_dashboardtile"."id", "posthog_dashboardtile"."dashboard_id", @@ -1003,7 +1123,41 @@ AND "posthog_dashboardtile"."insight_id" = 2) ''' # --- -# name: TestInsight.test_listing_insights_does_not_nplus1.18 +# name: TestInsight.test_listing_insights_does_not_nplus1.2 + ''' + SELECT "posthog_organizationmembership"."id", + "posthog_organizationmembership"."organization_id", + "posthog_organizationmembership"."user_id", + "posthog_organizationmembership"."level", + "posthog_organizationmembership"."joined_at", + "posthog_organizationmembership"."updated_at", + "posthog_organization"."id", + "posthog_organization"."name", + "posthog_organization"."slug", + "posthog_organization"."created_at", + "posthog_organization"."updated_at", + "posthog_organization"."plugins_access_level", + "posthog_organization"."for_internal_metrics", + "posthog_organization"."is_member_join_email_enabled", + "posthog_organization"."enforce_2fa", + "posthog_organization"."is_hipaa", + "posthog_organization"."customer_id", + "posthog_organization"."available_product_features", + "posthog_organization"."usage", + "posthog_organization"."never_drop_data", + "posthog_organization"."customer_trust_scores", + "posthog_organization"."setup_section_2_completed", + "posthog_organization"."personalization", + "posthog_organization"."domain_whitelist", + "posthog_organization"."available_features" + FROM "posthog_organizationmembership" + INNER JOIN "posthog_organization" ON ("posthog_organizationmembership"."organization_id" = "posthog_organization"."id") + WHERE ("posthog_organizationmembership"."organization_id" = '00000000-0000-0000-0000-000000000000'::uuid + AND "posthog_organizationmembership"."user_id" = 2) + LIMIT 21 + ''' +# --- +# name: TestInsight.test_listing_insights_does_not_nplus1.20 ''' SELECT "posthog_dashboardtile"."dashboard_id" FROM "posthog_dashboardtile" @@ -1014,7 +1168,7 @@ AND "posthog_dashboardtile"."insight_id" = 2) ''' # --- -# name: TestInsight.test_listing_insights_does_not_nplus1.19 +# name: TestInsight.test_listing_insights_does_not_nplus1.21 ''' SELECT "posthog_dashboard"."id", "posthog_dashboard"."name", @@ -1081,7 +1235,7 @@ WHERE NOT ("posthog_dashboarditem"."deleted") ''' # --- -# name: TestInsight.test_listing_insights_does_not_nplus1.21 +# name: TestInsight.test_listing_insights_does_not_nplus1.23 ''' SELECT "posthog_user"."id", "posthog_user"."password", @@ -1113,7 +1267,7 @@ LIMIT 21 ''' # --- -# name: TestInsight.test_listing_insights_does_not_nplus1.22 +# name: TestInsight.test_listing_insights_does_not_nplus1.24 ''' SELECT "posthog_team"."id", "posthog_team"."uuid", @@ -1172,7 +1326,87 @@ LIMIT 21 ''' # --- -# name: TestInsight.test_listing_insights_does_not_nplus1.23 +# name: TestInsight.test_listing_insights_does_not_nplus1.25 + ''' + SELECT "posthog_organizationmembership"."id", + "posthog_organizationmembership"."organization_id", + "posthog_organizationmembership"."user_id", + "posthog_organizationmembership"."level", + "posthog_organizationmembership"."joined_at", + "posthog_organizationmembership"."updated_at", + "posthog_organization"."id", + "posthog_organization"."name", + "posthog_organization"."slug", + "posthog_organization"."created_at", + "posthog_organization"."updated_at", + "posthog_organization"."plugins_access_level", + "posthog_organization"."for_internal_metrics", + "posthog_organization"."is_member_join_email_enabled", + "posthog_organization"."enforce_2fa", + "posthog_organization"."is_hipaa", + "posthog_organization"."customer_id", + "posthog_organization"."available_product_features", + "posthog_organization"."usage", + "posthog_organization"."never_drop_data", + "posthog_organization"."customer_trust_scores", + "posthog_organization"."setup_section_2_completed", + "posthog_organization"."personalization", + "posthog_organization"."domain_whitelist", + "posthog_organization"."available_features" + FROM "posthog_organizationmembership" + INNER JOIN "posthog_organization" ON ("posthog_organizationmembership"."organization_id" = "posthog_organization"."id") + WHERE ("posthog_organizationmembership"."organization_id" = '00000000-0000-0000-0000-000000000000'::uuid + AND "posthog_organizationmembership"."user_id" = 2) + LIMIT 21 + ''' +# --- +# name: TestInsight.test_listing_insights_does_not_nplus1.26 + ''' + SELECT "ee_accesscontrol"."id", + "ee_accesscontrol"."team_id", + "ee_accesscontrol"."resource", + "ee_accesscontrol"."access_level", + "ee_accesscontrol"."resource_id", + "ee_accesscontrol"."organization_member_id", + "ee_accesscontrol"."role_id", + "ee_accesscontrol"."created_by_id", + "ee_accesscontrol"."created_at", + "ee_accesscontrol"."updated_at" + FROM "ee_accesscontrol" + LEFT OUTER JOIN "posthog_organizationmembership" ON ("ee_accesscontrol"."organization_member_id" = "posthog_organizationmembership"."id") + WHERE (("ee_accesscontrol"."organization_member_id" IS NULL + AND "ee_accesscontrol"."resource" = 'project' + AND "ee_accesscontrol"."resource_id" = '420' + AND "ee_accesscontrol"."role_id" IS NULL + AND "ee_accesscontrol"."team_id" = 2) + OR ("posthog_organizationmembership"."user_id" = 2 + AND "ee_accesscontrol"."resource" = 'project' + AND "ee_accesscontrol"."resource_id" = '420' + AND "ee_accesscontrol"."role_id" IS NULL + AND "ee_accesscontrol"."team_id" = 2) + OR ("ee_accesscontrol"."organization_member_id" IS NULL + AND "ee_accesscontrol"."resource" = 'insight' + AND "ee_accesscontrol"."resource_id" IS NULL + AND "ee_accesscontrol"."role_id" IS NULL + AND "ee_accesscontrol"."team_id" = 2) + OR ("posthog_organizationmembership"."user_id" = 2 + AND "ee_accesscontrol"."resource" = 'insight' + AND "ee_accesscontrol"."resource_id" IS NULL + AND "ee_accesscontrol"."role_id" IS NULL + AND "ee_accesscontrol"."team_id" = 2) + OR ("ee_accesscontrol"."organization_member_id" IS NULL + AND "ee_accesscontrol"."resource" = 'insight' + AND "ee_accesscontrol"."resource_id" IS NOT NULL + AND "ee_accesscontrol"."role_id" IS NULL + AND "ee_accesscontrol"."team_id" = 2) + OR ("posthog_organizationmembership"."user_id" = 2 + AND "ee_accesscontrol"."resource" = 'insight' + AND "ee_accesscontrol"."resource_id" IS NOT NULL + AND "ee_accesscontrol"."role_id" IS NULL + AND "ee_accesscontrol"."team_id" = 2)) + ''' +# --- +# name: TestInsight.test_listing_insights_does_not_nplus1.27 ''' SELECT "posthog_organizationmembership"."id", "posthog_organizationmembership"."organization_id", @@ -1204,7 +1438,7 @@ WHERE "posthog_organizationmembership"."user_id" = 2 ''' # --- -# name: TestInsight.test_listing_insights_does_not_nplus1.24 +# name: TestInsight.test_listing_insights_does_not_nplus1.28 ''' SELECT "posthog_organization"."id", "posthog_organization"."name", @@ -1230,7 +1464,7 @@ LIMIT 21 ''' # --- -# name: TestInsight.test_listing_insights_does_not_nplus1.25 +# name: TestInsight.test_listing_insights_does_not_nplus1.29 ''' SELECT COUNT(*) AS "__count" FROM "posthog_dashboarditem" @@ -1239,7 +1473,53 @@ AND NOT ("posthog_dashboarditem"."deleted")) ''' # --- -# name: TestInsight.test_listing_insights_does_not_nplus1.26 +# name: TestInsight.test_listing_insights_does_not_nplus1.3 + ''' + SELECT "ee_accesscontrol"."id", + "ee_accesscontrol"."team_id", + "ee_accesscontrol"."resource", + "ee_accesscontrol"."access_level", + "ee_accesscontrol"."resource_id", + "ee_accesscontrol"."organization_member_id", + "ee_accesscontrol"."role_id", + "ee_accesscontrol"."created_by_id", + "ee_accesscontrol"."created_at", + "ee_accesscontrol"."updated_at" + FROM "ee_accesscontrol" + LEFT OUTER JOIN "posthog_organizationmembership" ON ("ee_accesscontrol"."organization_member_id" = "posthog_organizationmembership"."id") + WHERE (("ee_accesscontrol"."organization_member_id" IS NULL + AND "ee_accesscontrol"."resource" = 'project' + AND "ee_accesscontrol"."resource_id" = '420' + AND "ee_accesscontrol"."role_id" IS NULL + AND "ee_accesscontrol"."team_id" = 2) + OR ("posthog_organizationmembership"."user_id" = 2 + AND "ee_accesscontrol"."resource" = 'project' + AND "ee_accesscontrol"."resource_id" = '420' + AND "ee_accesscontrol"."role_id" IS NULL + AND "ee_accesscontrol"."team_id" = 2) + OR ("ee_accesscontrol"."organization_member_id" IS NULL + AND "ee_accesscontrol"."resource" = 'insight' + AND "ee_accesscontrol"."resource_id" IS NULL + AND "ee_accesscontrol"."role_id" IS NULL + AND "ee_accesscontrol"."team_id" = 2) + OR ("posthog_organizationmembership"."user_id" = 2 + AND "ee_accesscontrol"."resource" = 'insight' + AND "ee_accesscontrol"."resource_id" IS NULL + AND "ee_accesscontrol"."role_id" IS NULL + AND "ee_accesscontrol"."team_id" = 2) + OR ("ee_accesscontrol"."organization_member_id" IS NULL + AND "ee_accesscontrol"."resource" = 'insight' + AND "ee_accesscontrol"."resource_id" IS NOT NULL + AND "ee_accesscontrol"."role_id" IS NULL + AND "ee_accesscontrol"."team_id" = 2) + OR ("posthog_organizationmembership"."user_id" = 2 + AND "ee_accesscontrol"."resource" = 'insight' + AND "ee_accesscontrol"."resource_id" IS NOT NULL + AND "ee_accesscontrol"."role_id" IS NULL + AND "ee_accesscontrol"."team_id" = 2)) + ''' +# --- +# name: TestInsight.test_listing_insights_does_not_nplus1.30 ''' SELECT "posthog_dashboarditem"."id", "posthog_dashboarditem"."name", @@ -1390,7 +1670,7 @@ LIMIT 100 ''' # --- -# name: TestInsight.test_listing_insights_does_not_nplus1.27 +# name: TestInsight.test_listing_insights_does_not_nplus1.31 ''' SELECT ("posthog_dashboardtile"."insight_id") AS "_prefetch_related_val_insight_id", "posthog_dashboard"."id", @@ -1500,7 +1780,7 @@ 5 /* ... */)) ''' # --- -# name: TestInsight.test_listing_insights_does_not_nplus1.28 +# name: TestInsight.test_listing_insights_does_not_nplus1.32 ''' SELECT "posthog_dashboardtile"."id", "posthog_dashboardtile"."dashboard_id", @@ -1622,7 +1902,7 @@ 5 /* ... */)) ''' # --- -# name: TestInsight.test_listing_insights_does_not_nplus1.29 +# name: TestInsight.test_listing_insights_does_not_nplus1.33 ''' SELECT "posthog_taggeditem"."id", "posthog_taggeditem"."tag_id", @@ -1640,7 +1920,39 @@ 5 /* ... */) ''' # --- -# name: TestInsight.test_listing_insights_does_not_nplus1.3 +# name: TestInsight.test_listing_insights_does_not_nplus1.4 + ''' + SELECT "posthog_organizationmembership"."id", + "posthog_organizationmembership"."organization_id", + "posthog_organizationmembership"."user_id", + "posthog_organizationmembership"."level", + "posthog_organizationmembership"."joined_at", + "posthog_organizationmembership"."updated_at", + "posthog_organization"."id", + "posthog_organization"."name", + "posthog_organization"."slug", + "posthog_organization"."created_at", + "posthog_organization"."updated_at", + "posthog_organization"."plugins_access_level", + "posthog_organization"."for_internal_metrics", + "posthog_organization"."is_member_join_email_enabled", + "posthog_organization"."enforce_2fa", + "posthog_organization"."is_hipaa", + "posthog_organization"."customer_id", + "posthog_organization"."available_product_features", + "posthog_organization"."usage", + "posthog_organization"."never_drop_data", + "posthog_organization"."customer_trust_scores", + "posthog_organization"."setup_section_2_completed", + "posthog_organization"."personalization", + "posthog_organization"."domain_whitelist", + "posthog_organization"."available_features" + FROM "posthog_organizationmembership" + INNER JOIN "posthog_organization" ON ("posthog_organizationmembership"."organization_id" = "posthog_organization"."id") + WHERE "posthog_organizationmembership"."user_id" = 2 + ''' +# --- +# name: TestInsight.test_listing_insights_does_not_nplus1.5 ''' SELECT "posthog_dashboard"."id", "posthog_dashboard"."name", @@ -1742,7 +2054,7 @@ LIMIT 21 ''' # --- -# name: TestInsight.test_listing_insights_does_not_nplus1.5 +# name: TestInsight.test_listing_insights_does_not_nplus1.7 ''' SELECT "posthog_dashboarditem"."id", "posthog_dashboarditem"."name", @@ -1777,7 +2089,7 @@ LIMIT 21 ''' # --- -# name: TestInsight.test_listing_insights_does_not_nplus1.6 +# name: TestInsight.test_listing_insights_does_not_nplus1.8 ''' SELECT "posthog_team"."id", "posthog_team"."uuid", @@ -1843,7 +2155,7 @@ LIMIT 21 ''' # --- -# name: TestInsight.test_listing_insights_does_not_nplus1.7 +# name: TestInsight.test_listing_insights_does_not_nplus1.9 ''' SELECT "posthog_dashboard"."id", "posthog_dashboard"."name", diff --git a/posthog/api/test/__snapshots__/test_organization_feature_flag.ambr b/posthog/api/test/__snapshots__/test_organization_feature_flag.ambr index 9dffb44915b9f..e19696f7b6862 100644 --- a/posthog/api/test/__snapshots__/test_organization_feature_flag.ambr +++ b/posthog/api/test/__snapshots__/test_organization_feature_flag.ambr @@ -1993,6 +1993,40 @@ WHERE "posthog_earlyaccessfeature"."feature_flag_id" = 2 ''' # --- +# name: TestOrganizationFeatureFlagCopy.test_copy_feature_flag_create_new.73 + ''' + SELECT "posthog_organizationmembership"."id", + "posthog_organizationmembership"."organization_id", + "posthog_organizationmembership"."user_id", + "posthog_organizationmembership"."level", + "posthog_organizationmembership"."joined_at", + "posthog_organizationmembership"."updated_at", + "posthog_organization"."id", + "posthog_organization"."name", + "posthog_organization"."slug", + "posthog_organization"."created_at", + "posthog_organization"."updated_at", + "posthog_organization"."plugins_access_level", + "posthog_organization"."for_internal_metrics", + "posthog_organization"."is_member_join_email_enabled", + "posthog_organization"."enforce_2fa", + "posthog_organization"."is_hipaa", + "posthog_organization"."customer_id", + "posthog_organization"."available_product_features", + "posthog_organization"."usage", + "posthog_organization"."never_drop_data", + "posthog_organization"."customer_trust_scores", + "posthog_organization"."setup_section_2_completed", + "posthog_organization"."personalization", + "posthog_organization"."domain_whitelist", + "posthog_organization"."available_features" + FROM "posthog_organizationmembership" + INNER JOIN "posthog_organization" ON ("posthog_organizationmembership"."organization_id" = "posthog_organization"."id") + WHERE ("posthog_organizationmembership"."organization_id" = '00000000-0000-0000-0000-000000000000'::uuid + AND "posthog_organizationmembership"."user_id" = 2) + LIMIT 21 + ''' +# --- # name: TestOrganizationFeatureFlagCopy.test_copy_feature_flag_create_new.73 ''' SELECT "posthog_dashboard"."id", diff --git a/posthog/api/test/dashboards/test_dashboard.py b/posthog/api/test/dashboards/test_dashboard.py index ef97a1e6bd64b..53d2f289d9cb7 100644 --- a/posthog/api/test/dashboards/test_dashboard.py +++ b/posthog/api/test/dashboards/test_dashboard.py @@ -267,19 +267,21 @@ def test_adding_insights_is_not_nplus1_for_gets(self): "insight": "TRENDS", } - with self.assertNumQueries(11): + baseline = 4 + + with self.assertNumQueries(baseline + 10): self.dashboard_api.get_dashboard(dashboard_id, query_params={"no_items_field": "true"}) self.dashboard_api.create_insight({"filters": filter_dict, "dashboards": [dashboard_id]}) - with self.assertNumQueries(21): + with self.assertNumQueries(baseline + 10 + 10): self.dashboard_api.get_dashboard(dashboard_id, query_params={"no_items_field": "true"}) self.dashboard_api.create_insight({"filters": filter_dict, "dashboards": [dashboard_id]}) - with self.assertNumQueries(21): + with self.assertNumQueries(baseline + 10 + 10): self.dashboard_api.get_dashboard(dashboard_id, query_params={"no_items_field": "true"}) self.dashboard_api.create_insight({"filters": filter_dict, "dashboards": [dashboard_id]}) - with self.assertNumQueries(21): + with self.assertNumQueries(baseline + 10 + 10): self.dashboard_api.get_dashboard(dashboard_id, query_params={"no_items_field": "true"}) @snapshot_postgres_queries @@ -296,7 +298,7 @@ def test_listing_dashboards_is_not_nplus1(self) -> None: ) self.client.force_login(user_with_collaboration) - with self.assertNumQueries(7): + with self.assertNumQueries(10): self.dashboard_api.list_dashboards() for i in range(5): @@ -304,7 +306,7 @@ def test_listing_dashboards_is_not_nplus1(self) -> None: for j in range(3): self.dashboard_api.create_insight({"dashboards": [dashboard_id], "name": f"insight-{j}"}) - with self.assertNumQueries(FuzzyInt(8, 9)): + with self.assertNumQueries(FuzzyInt(10, 11)): self.dashboard_api.list_dashboards(query_params={"limit": 300}) def test_listing_dashboards_does_not_include_tiles(self) -> None: @@ -1339,6 +1341,7 @@ def test_create_from_template_json_can_provide_query_tile(self) -> None: "tags": [], "timezone": None, "updated_at": ANY, + "user_access_level": "editor", "hogql": ANY, "types": ANY, }, diff --git a/posthog/api/test/notebooks/test_notebook.py b/posthog/api/test/notebooks/test_notebook.py index f01d8fd6bc694..c3fa82861d949 100644 --- a/posthog/api/test/notebooks/test_notebook.py +++ b/posthog/api/test/notebooks/test_notebook.py @@ -95,6 +95,7 @@ def test_create_a_notebook(self, _, content: dict | None, text_content: str | No "deleted": False, "last_modified_at": mock.ANY, "last_modified_by": response.json()["last_modified_by"], + "user_access_level": "editor", } self.assert_notebook_activity( diff --git a/posthog/api/test/test_action.py b/posthog/api/test/test_action.py index 3a55bb84f9db5..fb0034e69b758 100644 --- a/posthog/api/test/test_action.py +++ b/posthog/api/test/test_action.py @@ -261,7 +261,7 @@ def test_update_action(self, patch_capture, *args): ) # test queries - with self.assertNumQueries(FuzzyInt(6, 8)): + with self.assertNumQueries(FuzzyInt(9, 11)): # Django session, user, team, org membership, instance setting, org, # count, action self.client.get(f"/api/projects/{self.team.id}/actions/") @@ -361,7 +361,7 @@ def test_listing_actions_is_not_nplus1(self) -> None: # Pre-query to cache things like instance settings self.client.get(f"/api/projects/{self.team.id}/actions/") - with self.assertNumQueries(6), snapshot_postgres_queries_context(self): + with self.assertNumQueries(9), snapshot_postgres_queries_context(self): self.client.get(f"/api/projects/{self.team.id}/actions/") Action.objects.create( @@ -370,7 +370,7 @@ def test_listing_actions_is_not_nplus1(self) -> None: created_by=User.objects.create_and_join(self.organization, "a", ""), ) - with self.assertNumQueries(6), snapshot_postgres_queries_context(self): + with self.assertNumQueries(9), snapshot_postgres_queries_context(self): self.client.get(f"/api/projects/{self.team.id}/actions/") Action.objects.create( @@ -379,7 +379,7 @@ def test_listing_actions_is_not_nplus1(self) -> None: created_by=User.objects.create_and_join(self.organization, "b", ""), ) - with self.assertNumQueries(6), snapshot_postgres_queries_context(self): + with self.assertNumQueries(9), snapshot_postgres_queries_context(self): self.client.get(f"/api/projects/{self.team.id}/actions/") def test_get_tags_on_non_ee_returns_empty_list(self): diff --git a/posthog/api/test/test_activity_log.py b/posthog/api/test/test_activity_log.py index 2edd7219b71a3..738dae72a64a3 100644 --- a/posthog/api/test/test_activity_log.py +++ b/posthog/api/test/test_activity_log.py @@ -298,7 +298,7 @@ def test_notifications_viewed_n_plus_1(self) -> None: user=user, defaults={"last_viewed_activity_date": f"2023-0{i}-17T04:36:50Z"} ) - with self.assertNumQueries(FuzzyInt(39, 39)): + with self.assertNumQueries(FuzzyInt(40, 40)): self.client.get(f"/api/projects/{self.team.id}/activity_log/important_changes") def test_can_list_all_activity(self) -> None: diff --git a/posthog/api/test/test_annotation.py b/posthog/api/test/test_annotation.py index 0a1f2396d42d9..842f2dde432d5 100644 --- a/posthog/api/test/test_annotation.py +++ b/posthog/api/test/test_annotation.py @@ -36,7 +36,7 @@ def test_retrieving_annotation_is_not_n_plus_1(self, _mock_capture) -> None: """ see https://sentry.io/organizations/posthog/issues/3706110236/events/db0167ece56649f59b013cbe9de7ba7a/?project=1899813 """ - with self.assertNumQueries(FuzzyInt(6, 7)), snapshot_postgres_queries_context(self): + with self.assertNumQueries(FuzzyInt(8, 9)), snapshot_postgres_queries_context(self): response = self.client.get(f"/api/projects/{self.team.id}/annotations/").json() self.assertEqual(len(response["results"]), 0) @@ -48,7 +48,7 @@ def test_retrieving_annotation_is_not_n_plus_1(self, _mock_capture) -> None: content=now().isoformat(), ) - with self.assertNumQueries(FuzzyInt(6, 7)), snapshot_postgres_queries_context(self): + with self.assertNumQueries(FuzzyInt(8, 9)), snapshot_postgres_queries_context(self): response = self.client.get(f"/api/projects/{self.team.id}/annotations/").json() self.assertEqual(len(response["results"]), 1) @@ -60,7 +60,7 @@ def test_retrieving_annotation_is_not_n_plus_1(self, _mock_capture) -> None: content=now().isoformat(), ) - with self.assertNumQueries(FuzzyInt(6, 7)), snapshot_postgres_queries_context(self): + with self.assertNumQueries(FuzzyInt(8, 9)), snapshot_postgres_queries_context(self): response = self.client.get(f"/api/projects/{self.team.id}/annotations/").json() self.assertEqual(len(response["results"]), 2) diff --git a/posthog/api/test/test_cohort.py b/posthog/api/test/test_cohort.py index 52dea5f41a9e0..b6002e81882a9 100644 --- a/posthog/api/test/test_cohort.py +++ b/posthog/api/test/test_cohort.py @@ -241,7 +241,7 @@ def test_list_cohorts_is_not_nplus1(self, patch_calculate_cohort, patch_capture) ) self.assertEqual(response.status_code, 201, response.content) - with self.assertNumQueries(9): + with self.assertNumQueries(11): response = self.client.get(f"/api/projects/{self.team.id}/cohorts") assert len(response.json()["results"]) == 1 @@ -256,7 +256,7 @@ def test_list_cohorts_is_not_nplus1(self, patch_calculate_cohort, patch_capture) ) self.assertEqual(response.status_code, 201, response.content) - with self.assertNumQueries(9): + with self.assertNumQueries(12): response = self.client.get(f"/api/projects/{self.team.id}/cohorts") assert len(response.json()["results"]) == 3 diff --git a/posthog/api/test/test_event.py b/posthog/api/test/test_event.py index 595d55e9ba48c..65019511b5758 100644 --- a/posthog/api/test/test_event.py +++ b/posthog/api/test/test_event.py @@ -97,7 +97,7 @@ def test_filter_events_by_event_name(self): # Django session, PostHog user, PostHog team, PostHog org membership, # instance setting check, person and distinct id - with self.assertNumQueries(7): + with self.assertNumQueries(9): response = self.client.get(f"/api/projects/{self.team.id}/events/?event=event_name").json() self.assertEqual(response["results"][0]["event"], "event_name") @@ -125,7 +125,7 @@ def test_filter_events_by_properties(self): # Django session, PostHog user, PostHog team, PostHog org membership, # look up if rate limit is enabled (cached after first lookup), instance # setting (poe, rate limit), person and distinct id - expected_queries = 8 + expected_queries = 10 with self.assertNumQueries(expected_queries): response = self.client.get( diff --git a/posthog/api/test/test_feature_flag.py b/posthog/api/test/test_feature_flag.py index f43e49aaee91c..c6f2deeb100a7 100644 --- a/posthog/api/test/test_feature_flag.py +++ b/posthog/api/test/test_feature_flag.py @@ -1240,7 +1240,7 @@ def test_my_flags_is_not_nplus1(self) -> None: format="json", ).json() - with self.assertNumQueries(FuzzyInt(5, 6)): + with self.assertNumQueries(FuzzyInt(7, 8)): response = self.client.get(f"/api/projects/{self.team.id}/feature_flags/my_flags") self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -1255,7 +1255,7 @@ def test_my_flags_is_not_nplus1(self) -> None: format="json", ).json() - with self.assertNumQueries(FuzzyInt(5, 6)): + with self.assertNumQueries(FuzzyInt(7, 8)): response = self.client.get(f"/api/projects/{self.team.id}/feature_flags/my_flags") self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -1270,7 +1270,7 @@ def test_getting_flags_is_not_nplus1(self) -> None: format="json", ).json() - with self.assertNumQueries(FuzzyInt(11, 12)): + with self.assertNumQueries(FuzzyInt(15, 16)): response = self.client.get(f"/api/projects/{self.team.id}/feature_flags") self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -1285,7 +1285,7 @@ def test_getting_flags_is_not_nplus1(self) -> None: format="json", ).json() - with self.assertNumQueries(FuzzyInt(11, 12)): + with self.assertNumQueries(FuzzyInt(15, 16)): response = self.client.get(f"/api/projects/{self.team.id}/feature_flags") self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -1309,7 +1309,7 @@ def test_getting_flags_with_no_creator(self) -> None: name="Flag role access", ) - with self.assertNumQueries(FuzzyInt(11, 12)): + with self.assertNumQueries(FuzzyInt(15, 16)): response = self.client.get(f"/api/projects/{self.team.id}/feature_flags") self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(len(response.json()["results"]), 2) @@ -2219,19 +2219,23 @@ def test_local_evaluation_for_invalid_cohorts(self, mock_capture): self.client.logout() - with self.assertNumQueries(12): - # E 1. SAVEPOINT - # E 2. SELECT "posthog_personalapikey"."id" - # E 3. RELEASE SAVEPOINT - # E 4. UPDATE "posthog_personalapikey" SET "last_used_at" = '2024-01-31T13:01:37.394080+00:00' - # E 5. SELECT "posthog_team"."id", "posthog_team"."uuid" - # E 6. SELECT "posthog_organizationmembership"."id", "posthog_organizationmembership"."organization_id" - # E 7. SELECT "posthog_cohort"."id" -- all cohorts - # E 8. SELECT "posthog_featureflag"."id", "posthog_featureflag"."key", -- all flags - # E 9. SELECT "posthog_cohort". id = 99999 - # E 10. SELECT "posthog_cohort". id = deleted cohort - # E 11. SELECT "posthog_cohort". id = cohort from other team - # E 12. SELECT "posthog_grouptypemapping"."id", -- group type mapping + with self.assertNumQueries(16): + # 1. SAVEPOINT + # 2. SELECT "posthog_personalapikey"."id", + # 3. RELEASE SAVEPOINT + # 4. UPDATE "posthog_personalapikey" SET "last_used_at" + # 5. SELECT "posthog_team"."id", "posthog_team"."uuid", + # 6. SELECT "posthog_team"."id", "posthog_team"."uuid", + # 7. SELECT "posthog_project"."id", "posthog_project"."organization_id", + # 8. SELECT "posthog_organizationmembership"."id", + # 9. SELECT "ee_accesscontrol"."id", + # 10. SELECT "posthog_organizationmembership"."id", + # 11. SELECT "posthog_cohort"."id" -- all cohorts + # 12. SELECT "posthog_featureflag"."id", "posthog_featureflag"."key", -- all flags + # 13. SELECT "posthog_cohort". id = 99999 + # 14. SELECT "posthog_cohort". id = deleted cohort + # 15. SELECT "posthog_cohort". id = cohort from other team + # 16. SELECT "posthog_grouptypemapping"."id", -- group type mapping response = self.client.get( f"/api/feature_flag/local_evaluation?token={self.team.api_token}&send_cohorts", diff --git a/posthog/api/test/test_insight.py b/posthog/api/test/test_insight.py index 32cd4da9df83f..3f738497dd5de 100644 --- a/posthog/api/test/test_insight.py +++ b/posthog/api/test/test_insight.py @@ -505,11 +505,11 @@ def test_listing_insights_does_not_nplus1(self) -> None: # adding more insights doesn't change the query count self.assertEqual( [ - FuzzyInt(10, 11), - FuzzyInt(10, 11), - FuzzyInt(10, 11), - FuzzyInt(10, 11), - FuzzyInt(10, 11), + FuzzyInt(12, 13), + FuzzyInt(12, 13), + FuzzyInt(12, 13), + FuzzyInt(12, 13), + FuzzyInt(12, 13), ], query_counts, f"received query counts\n\n{query_counts}", diff --git a/posthog/api/test/test_organization_feature_flag.py b/posthog/api/test/test_organization_feature_flag.py index 83550c689d5ae..7400088231ca3 100644 --- a/posthog/api/test/test_organization_feature_flag.py +++ b/posthog/api/test/test_organization_feature_flag.py @@ -3,7 +3,7 @@ from rest_framework import status -from ee.models.organization_resource_access import OrganizationResourceAccess +from ee.models.rbac.organization_resource_access import OrganizationResourceAccess from posthog.api.dashboards.dashboard import Dashboard from posthog.constants import AvailableFeature from posthog.models import FeatureFlag @@ -115,10 +115,10 @@ def test_copy_feature_flag_create_new(self): "ensure_experience_continuity": self.feature_flag_to_copy.ensure_experience_continuity, "rollout_percentage": self.rollout_percentage_to_copy, "deleted": False, - "created_by": self.user.id, - "id": "__ignore__", - "created_at": "__ignore__", - "usage_dashboard": "__ignore__", + "created_by": ANY, + "id": ANY, + "created_at": ANY, + "usage_dashboard": ANY, "is_simple_flag": True, "experiment_set": [], "surveys": [], @@ -129,22 +129,13 @@ def test_copy_feature_flag_create_new(self): "analytics_dashboards": [], "has_enriched_analytics": False, "tags": [], + "user_access_level": "editor", } flag_response = response.json()["success"][0] - for key, expected_value in expected_flag_response.items(): - self.assertIn(key, flag_response) - if expected_value != "__ignore__": - if key == "created_by": - self.assertEqual(flag_response[key]["id"], expected_value) - else: - self.assertEqual(flag_response[key], expected_value) - - self.assertSetEqual( - set(expected_flag_response.keys()), - set(flag_response.keys()), - ) + assert flag_response == expected_flag_response + assert flag_response["created_by"]["id"] == self.user.id def test_copy_feature_flag_update_existing(self): target_project = self.team_2 @@ -201,43 +192,32 @@ def test_copy_feature_flag_update_existing(self): "ensure_experience_continuity": self.feature_flag_to_copy.ensure_experience_continuity, "rollout_percentage": self.rollout_percentage_to_copy, "deleted": False, - "created_by": self.user.id, + "created_by": ANY, "is_simple_flag": True, "rollback_conditions": None, "performed_rollback": False, "can_edit": True, "has_enriched_analytics": False, "tags": [], - "id": "__ignore__", - "created_at": "__ignore__", - "usage_dashboard": "__ignore__", - "experiment_set": "__ignore__", - "surveys": "__ignore__", - "features": "__ignore__", - "analytics_dashboards": "__ignore__", + "id": ANY, + "created_at": ANY, + "usage_dashboard": ANY, + "experiment_set": ANY, + "surveys": ANY, + "features": ANY, + "analytics_dashboards": ANY, + "user_access_level": "editor", } flag_response = response.json()["success"][0] - - for key, expected_value in expected_flag_response.items(): - self.assertIn(key, flag_response) - if expected_value != "__ignore__": - if key == "created_by": - self.assertEqual(flag_response[key]["id"], expected_value) - else: - self.assertEqual(flag_response[key], expected_value) - + assert flag_response == expected_flag_response # Linked instances must remain linked - self.assertEqual(experiment.id, flag_response["experiment_set"][0]) - self.assertEqual(str(survey.id), flag_response["surveys"][0]["id"]) - self.assertEqual(str(feature.id), flag_response["features"][0]["id"]) - self.assertEqual(analytics_dashboard.id, flag_response["analytics_dashboards"][0]) - self.assertEqual(usage_dashboard.id, flag_response["usage_dashboard"]) - - self.assertSetEqual( - set(expected_flag_response.keys()), - set(flag_response.keys()), - ) + assert flag_response["created_by"]["id"] == self.user.id + assert experiment.id == flag_response["experiment_set"][0] + assert str(survey.id) == flag_response["surveys"][0]["id"] + assert str(feature.id) == flag_response["features"][0]["id"] + assert analytics_dashboard.id == flag_response["analytics_dashboards"][0] + assert usage_dashboard.id == flag_response["usage_dashboard"] def test_copy_feature_flag_with_old_legacy_flags(self): url = f"/api/organizations/{self.organization.id}/feature_flags/copy_flags" @@ -331,30 +311,25 @@ def test_copy_feature_flag_update_override_deleted(self): "ensure_experience_continuity": self.feature_flag_to_copy.ensure_experience_continuity, "rollout_percentage": self.rollout_percentage_to_copy, "deleted": False, - "created_by": self.user.id, + "created_by": ANY, "is_simple_flag": True, "rollback_conditions": None, "performed_rollback": False, "can_edit": True, "has_enriched_analytics": False, "tags": [], - "id": "__ignore__", - "created_at": "__ignore__", - "usage_dashboard": "__ignore__", - "experiment_set": "__ignore__", - "surveys": "__ignore__", - "features": "__ignore__", - "analytics_dashboards": "__ignore__", + "id": ANY, + "created_at": ANY, + "usage_dashboard": ANY, + "experiment_set": ANY, + "surveys": ANY, + "features": ANY, + "analytics_dashboards": ANY, + "user_access_level": "editor", } flag_response = response.json()["success"][0] - - for key, expected_value in expected_flag_response.items(): - self.assertIn(key, flag_response) - if expected_value != "__ignore__": - if key == "created_by": - self.assertEqual(flag_response[key]["id"], expected_value) - else: - self.assertEqual(flag_response[key], expected_value) + assert flag_response == expected_flag_response + assert flag_response["created_by"]["id"] == self.user.id # Linked instances must be overriden for a soft-deleted flag self.assertEqual(flag_response["experiment_set"], []) @@ -362,11 +337,6 @@ def test_copy_feature_flag_update_override_deleted(self): self.assertNotEqual(flag_response["usage_dashboard"], existing_deleted_flag.usage_dashboard.id) self.assertEqual(flag_response["analytics_dashboards"], []) - self.assertSetEqual( - set(expected_flag_response.keys()), - set(flag_response.keys()), - ) - # target_project_2 should have failed self.assertEqual(len(response.json()["failed"]), 1) self.assertEqual(response.json()["failed"][0]["project_id"], target_project_2.id) diff --git a/posthog/api/test/test_person.py b/posthog/api/test/test_person.py index fd2c6e042a15f..29eb3990407d5 100644 --- a/posthog/api/test/test_person.py +++ b/posthog/api/test/test_person.py @@ -873,7 +873,7 @@ def test_pagination_limit(self): create_person(team_id=self.team.pk, version=0) returned_ids = [] - with self.assertNumQueries(7): + with self.assertNumQueries(9): response = self.client.get("/api/person/?limit=10").json() self.assertEqual(len(response["results"]), 9) returned_ids += [x["distinct_ids"][0] for x in response["results"]] @@ -884,7 +884,7 @@ def test_pagination_limit(self): created_ids.reverse() # ids are returned in desc order self.assertEqual(returned_ids, created_ids, returned_ids) - with self.assertNumQueries(6): + with self.assertNumQueries(8): response_include_total = self.client.get("/api/person/?limit=10&include_total").json() self.assertEqual(response_include_total["count"], 20) # With `include_total`, the total count is returned too diff --git a/posthog/api/test/test_plugin.py b/posthog/api/test/test_plugin.py index f92350045a38c..be947c9662909 100644 --- a/posthog/api/test/test_plugin.py +++ b/posthog/api/test/test_plugin.py @@ -955,22 +955,22 @@ def test_can_access_global_plugin_even_if_not_in_org(self, mock_get, mock_reload @snapshot_postgres_queries def test_listing_plugins_is_not_nplus1(self, _mock_get, _mock_reload) -> None: - with self.assertNumQueries(8): + with self.assertNumQueries(10): self._assert_number_of_when_listed_plugins(0) Plugin.objects.create(organization=self.organization) - with self.assertNumQueries(8): + with self.assertNumQueries(10): self._assert_number_of_when_listed_plugins(1) Plugin.objects.create(organization=self.organization) - with self.assertNumQueries(8): + with self.assertNumQueries(10): self._assert_number_of_when_listed_plugins(2) Plugin.objects.create(organization=self.organization) - with self.assertNumQueries(8): + with self.assertNumQueries(10): self._assert_number_of_when_listed_plugins(3) def _assert_number_of_when_listed_plugins(self, expected_plugins_count: int) -> None: diff --git a/posthog/api/test/test_survey.py b/posthog/api/test/test_survey.py index c6de95a44702e..b3daf99640a19 100644 --- a/posthog/api/test/test_survey.py +++ b/posthog/api/test/test_survey.py @@ -336,7 +336,7 @@ def test_used_in_survey_is_populated_correctly_for_feature_flag_list(self) -> No format="json", ).json() - with self.assertNumQueries(16): + with self.assertNumQueries(20): response = self.client.get(f"/api/projects/{self.team.id}/feature_flags") self.assertEqual(response.status_code, status.HTTP_200_OK) result = response.json() diff --git a/posthog/hogql/transforms/test/__snapshots__/test_in_cohort.ambr b/posthog/hogql/transforms/test/__snapshots__/test_in_cohort.ambr index f018e96ef067a..369f18ab9d118 100644 --- a/posthog/hogql/transforms/test/__snapshots__/test_in_cohort.ambr +++ b/posthog/hogql/transforms/test/__snapshots__/test_in_cohort.ambr @@ -31,7 +31,7 @@ FROM events LEFT JOIN ( SELECT person_static_cohort.person_id AS cohort_person_id, 1 AS matched, person_static_cohort.cohort_id AS cohort_id FROM person_static_cohort - WHERE and(equals(person_static_cohort.team_id, 420), in(person_static_cohort.cohort_id, [4]))) AS __in_cohort ON equals(__in_cohort.cohort_person_id, events.person_id) + WHERE and(equals(person_static_cohort.team_id, 420), in(person_static_cohort.cohort_id, [2]))) AS __in_cohort ON equals(__in_cohort.cohort_person_id, events.person_id) WHERE and(equals(events.team_id, 420), 1, ifNull(equals(__in_cohort.matched, 1), 0)) LIMIT 100 SETTINGS readonly=2, max_execution_time=60, allow_experimental_object_type=1, format_csv_allow_double_quotes=0, max_ast_elements=4000000, max_expanded_ast_elements=4000000, max_bytes_before_external_group_by=0 @@ -42,7 +42,7 @@ FROM events LEFT JOIN ( SELECT person_id AS cohort_person_id, 1 AS matched, cohort_id FROM static_cohort_people - WHERE in(cohort_id, [4])) AS __in_cohort ON equals(__in_cohort.cohort_person_id, person_id) + WHERE in(cohort_id, [2])) AS __in_cohort ON equals(__in_cohort.cohort_person_id, person_id) WHERE and(1, equals(__in_cohort.matched, 1)) LIMIT 100 ''' @@ -55,7 +55,7 @@ FROM events LEFT JOIN ( SELECT person_static_cohort.person_id AS cohort_person_id, 1 AS matched, person_static_cohort.cohort_id AS cohort_id FROM person_static_cohort - WHERE and(equals(person_static_cohort.team_id, 420), in(person_static_cohort.cohort_id, [5]))) AS __in_cohort ON equals(__in_cohort.cohort_person_id, events.person_id) + WHERE and(equals(person_static_cohort.team_id, 420), in(person_static_cohort.cohort_id, [3]))) AS __in_cohort ON equals(__in_cohort.cohort_person_id, events.person_id) WHERE and(equals(events.team_id, 420), 1, ifNull(equals(__in_cohort.matched, 1), 0)) LIMIT 100 SETTINGS readonly=2, max_execution_time=60, allow_experimental_object_type=1, format_csv_allow_double_quotes=0, max_ast_elements=4000000, max_expanded_ast_elements=4000000, max_bytes_before_external_group_by=0 @@ -66,7 +66,7 @@ FROM events LEFT JOIN ( SELECT person_id AS cohort_person_id, 1 AS matched, cohort_id FROM static_cohort_people - WHERE in(cohort_id, [5])) AS __in_cohort ON equals(__in_cohort.cohort_person_id, person_id) + WHERE in(cohort_id, [3])) AS __in_cohort ON equals(__in_cohort.cohort_person_id, person_id) WHERE and(1, equals(__in_cohort.matched, 1)) LIMIT 100 ''' diff --git a/posthog/middleware.py b/posthog/middleware.py index 71b5e2563e084..782931cfa8076 100644 --- a/posthog/middleware.py +++ b/posthog/middleware.py @@ -36,6 +36,7 @@ from posthog.metrics import LABEL_TEAM_ID from posthog.models import Action, Cohort, Dashboard, FeatureFlag, Insight, Notebook, User, Team from posthog.rate_limit import DecideRateThrottle +from posthog.rbac.user_access_control import UserAccessControl from posthog.settings import SITE_URL, DEBUG, PROJECT_SWITCHING_TOKEN_ALLOWLIST from posthog.user_permissions import UserPermissions from .auth import PersonalAPIKeyAuthentication @@ -274,6 +275,14 @@ def switch_team_if_allowed(self, new_team: Team, request: HttpRequest): def can_switch_to_team(self, new_team: Team, request: HttpRequest): user = cast(User, request.user) user_permissions = UserPermissions(user) + user_access_control = UserAccessControl(user=user, team=new_team) + + # :KLUDGE: This is more inefficient than needed, doing several expensive lookups + # However this should be a rare operation! + if not user_access_control.check_access_level_for_object(new_team, "member"): + # Do something to indicate that they don't have access to the team... + return False + # :KLUDGE: This is more inefficient than needed, doing several expensive lookups # However this should be a rare operation! if user_permissions.team(new_team).effective_membership_level is None: diff --git a/posthog/models/feature_flag/permissions.py b/posthog/models/feature_flag/permissions.py index 8f766b4fccc60..d2b0bb858a403 100644 --- a/posthog/models/feature_flag/permissions.py +++ b/posthog/models/feature_flag/permissions.py @@ -6,7 +6,7 @@ def can_user_edit_feature_flag(request, feature_flag): # self hosted check for enterprise models that may not exist try: from ee.models.feature_flag_role_access import FeatureFlagRoleAccess - from ee.models.organization_resource_access import OrganizationResourceAccess + from ee.models.rbac.organization_resource_access import OrganizationResourceAccess except: return True else: diff --git a/posthog/models/personal_api_key.py b/posthog/models/personal_api_key.py index ea886b55757c6..cac5adefbc65d 100644 --- a/posthog/models/personal_api_key.py +++ b/posthog/models/personal_api_key.py @@ -1,4 +1,4 @@ -from typing import Optional, Literal, get_args +from typing import Optional, Literal import hashlib from django.contrib.auth.hashers import PBKDF2PasswordHasher @@ -66,56 +66,3 @@ class PersonalAPIKey(models.Model): null=True, blank=True, ) - - -## API Scopes -# These are the scopes that are used to define the permissions of the API tokens. -# Not every model needs a scope - it should more be for top-level things -# Typically each object should have `read` and `write` scopes, but some objects may have more specific scopes - -# WARNING: Make sure to keep in sync with the frontend! -APIScopeObject = Literal[ - "action", - "activity_log", - "annotation", - "batch_export", - "cohort", - "dashboard", - "dashboard_template", - "early_access_feature", - "event_definition", - "experiment", - "export", - "feature_flag", - "group", - "insight", - "query", # Covers query and events endpoints - "notebook", - "organization", - "organization_member", - "person", - "plugin", - "project", - "property_definition", - "session_recording", - "session_recording_playlist", - "sharing_configuration", - "subscription", - "survey", - "user", - "webhook", -] - -APIScopeActions = Literal[ - "read", - "write", -] - -APIScopeObjectOrNotSupported = Literal[ - APIScopeObject, - "INTERNAL", -] - - -API_SCOPE_OBJECTS: tuple[APIScopeObject, ...] = get_args(APIScopeObject) -API_SCOPE_ACTIONS: tuple[APIScopeActions, ...] = get_args(APIScopeActions) diff --git a/posthog/models/scopes.py b/posthog/models/scopes.py new file mode 100644 index 0000000000000..2bd0d48b405e5 --- /dev/null +++ b/posthog/models/scopes.py @@ -0,0 +1,60 @@ +## API Scopes +# These are the scopes that are used to define the permissions of the API tokens. +# Not every model needs a scope - it should more be for top-level things +# Typically each object should have `read` and `write` scopes, but some objects may have more specific scopes + +# WARNING: Make sure to keep in sync with the frontend! +from typing import Literal, get_args + + +## API Scopes +# These are the scopes that are used to define the permissions of the API tokens. +# Not every model needs a scope - it should more be for top-level things +# Typically each object should have `read` and `write` scopes, but some objects may have more specific scopes + +# WARNING: Make sure to keep in sync with the frontend! +APIScopeObject = Literal[ + "action", + "activity_log", + "annotation", + "batch_export", + "cohort", + "dashboard", + "dashboard_template", + "early_access_feature", + "event_definition", + "experiment", + "export", + "feature_flag", + "group", + "insight", + "query", # Covers query and events endpoints + "notebook", + "organization", + "organization_member", + "person", + "plugin", + "project", + "property_definition", + "session_recording", + "session_recording_playlist", + "sharing_configuration", + "subscription", + "survey", + "user", + "webhook", +] + +APIScopeActions = Literal[ + "read", + "write", +] + +APIScopeObjectOrNotSupported = Literal[ + APIScopeObject, + "INTERNAL", +] + + +API_SCOPE_OBJECTS: tuple[APIScopeObject, ...] = get_args(APIScopeObject) +API_SCOPE_ACTIONS: tuple[APIScopeActions, ...] = get_args(APIScopeActions) diff --git a/posthog/permissions.py b/posthog/permissions.py index 889832c13baa7..6dff5453fdd83 100644 --- a/posthog/permissions.py +++ b/posthog/permissions.py @@ -1,15 +1,15 @@ +from typing import Optional, cast import time -from typing import cast from django.conf import settings from django.core.exceptions import ImproperlyConfigured from django.db.models import Model -from django.views import View import posthoganalytics from rest_framework.exceptions import NotFound, PermissionDenied from rest_framework.permissions import SAFE_METHODS, BasePermission, IsAdminUser from rest_framework.request import Request from rest_framework.views import APIView +from rest_framework.viewsets import ViewSet from posthog.auth import ( PersonalAPIKeyAuthentication, @@ -19,13 +19,38 @@ from posthog.cloud_utils import is_cloud from posthog.exceptions import EnterpriseFeatureException from posthog.models import Organization, OrganizationMembership, Team, User -from posthog.models.personal_api_key import APIScopeObjectOrNotSupported +from posthog.models.scopes import APIScopeObject, APIScopeObjectOrNotSupported +from posthog.rbac.user_access_control import AccessControlLevel, UserAccessControl, ordered_access_levels from posthog.utils import get_can_create_org -CREATE_METHODS = ["POST", "PUT"] +CREATE_ACTIONS = ["create", "update"] -def extract_organization(object: Model, view: View) -> Organization: +def extract_organization(object: Model, view: ViewSet) -> Organization: + # This is set as part of the TeamAndOrgViewSetMixin to allow models that are not directly related to an organization + organization_id_rewrite = getattr(view, "filter_rewrite_rules", {}).get("organization_id") + if organization_id_rewrite: + for part in organization_id_rewrite.split("__"): + if part == "organization_id": + break + object = getattr(object, part) + + if isinstance(object, Organization): + return object + try: + return object.organization # type: ignore + except AttributeError: + try: + return object.team.organization # type: ignore + except AttributeError: + try: + return object.project.organization # type: ignore + except AttributeError: + pass + raise ValueError("Object not compatible with organization-based permissions!") + + +def extract_organization_membership(object: Model, view: ViewSet) -> Organization: # This is set as part of the TeamAndOrgViewSetMixin to allow models that are not directly related to an organization organization_id_rewrite = getattr(view, "filter_rewrite_rules", {}).get("organization_id") if organization_id_rewrite: @@ -101,10 +126,14 @@ def has_permission(self, request: Request, view) -> bool: organization = get_organization_from_view(view) + # TODO: Optimize this - we can get it from view.user_access_control + return OrganizationMembership.objects.filter(user=cast(User, request.user), organization=organization).exists() - def has_object_permission(self, request: Request, view: View, object: Model) -> bool: + def has_object_permission(self, request: Request, view, object: Model) -> bool: organization = extract_organization(object, view) + + # TODO: Optimize this - we can get it from view.user_access_control return OrganizationMembership.objects.filter(user=cast(User, request.user), organization=organization).exists() @@ -135,7 +164,7 @@ def has_permission(self, request: Request, view) -> bool: return membership.level >= OrganizationMembership.Level.ADMIN - def has_object_permission(self, request: Request, view: View, object: Model) -> bool: + def has_object_permission(self, request: Request, view, object: Model) -> bool: if request.method in SAFE_METHODS: return True @@ -295,15 +324,9 @@ def has_permission(self, request, view) -> bool: return True -class APIScopePermission(BasePermission): +class ScopeBasePermission(BasePermission): """ - The request is via an API key and the user has the appropriate scopes. - - This permission requires that the view has a "scope" attribute which is the base scope required for the action. - E.g. scope="insight" for a view that requires "insight:read" or "insight:write" for the relevant actions. - - Actions can override this default scope by setting the `required_scopes` attribute on the view method. - + Base class for shared functionality between APIScopePermission and AccessControlPermission """ write_actions: list[str] = ["create", "update", "partial_update", "patch", "destroy"] @@ -311,18 +334,57 @@ class APIScopePermission(BasePermission): scope_object_read_actions: list[str] = [] scope_object_write_actions: list[str] = [] + def _get_scope_object(self, request, view) -> APIScopeObjectOrNotSupported: + if not getattr(view, "scope_object", None): + raise ImproperlyConfigured("APIScopePermission requires the view to define the scope_object attribute.") + + return view.scope_object + def _get_action(self, request, view) -> str: # TRICKY: DRF doesn't have an action for non-detail level "patch" calls which we use sometimes - if not view.action: if request.method == "PATCH" and not view.detail: return "patch" return view.action + def _get_required_scopes(self, request, view) -> Optional[list[str]]: + # If required_scopes is set on the view method then use that + # Otherwise use the scope_object and derive the required scope from the action + if getattr(view, "required_scopes", None): + return view.required_scopes + + scope_object = self._get_scope_object(request, view) + + if scope_object == "INTERNAL": + return None + + action = self._get_action(request, view) + read_actions = getattr(view, "scope_object_read_actions", self.read_actions) + write_actions = getattr(view, "scope_object_write_actions", self.write_actions) + + if action in write_actions: + return [f"{scope_object}:write"] + elif action in read_actions or request.method == "OPTIONS": + return [f"{scope_object}:read"] + + return None + + +class APIScopePermission(ScopeBasePermission): + """ + The request is via an API key and the user has the appropriate scopes. + + This permission requires that the view has a "scope" attribute which is the base scope required for the action. + E.g. scope="insight" for a view that requires "insight:read" or "insight:write" for the relevant actions. + + Actions can override this default scope by setting the `required_scopes` attribute on the view method. + + """ + def has_permission(self, request, view) -> bool: # NOTE: We do this first to error out quickly if the view is missing the required attribute # Helps devs remember to add it. - self.get_scope_object(request, view) + self._get_scope_object(request, view) # API Scopes currently only apply to PersonalAPIKeyAuthentication if not isinstance(request.successful_authenticator, PersonalAPIKeyAuthentication): @@ -334,7 +396,12 @@ def has_permission(self, request, view) -> bool: if not key_scopes: return True - required_scopes = self.get_required_scopes(request, view) + required_scopes = self._get_required_scopes(request, view) + + if not required_scopes: + self.message = f"This action does not support Personal API Key access" + return False + self.check_team_and_org_permissions(request, view) if "*" in key_scopes: @@ -354,7 +421,7 @@ def has_permission(self, request, view) -> bool: return True def check_team_and_org_permissions(self, request, view) -> None: - scope_object = self.get_scope_object(request, view) + scope_object = self._get_scope_object(request, view) if scope_object == "user": return # The /api/users/@me/ endpoint is exempt from team and org scoping @@ -380,35 +447,104 @@ def check_team_and_org_permissions(self, request, view) -> None: # Indicates this is not an organization scoped view pass - def get_required_scopes(self, request, view) -> list[str]: - # If required_scopes is set on the view method then use that - # Otherwise use the scope_object and derive the required scope from the action - if getattr(view, "required_scopes", None): - return view.required_scopes - scope_object = self.get_scope_object(request, view) +class AccessControlPermission(ScopeBasePermission): + """ + Unified permissions access - controls access to any object based on the user's access controls + """ + + def _get_user_access_control(self, request, view) -> UserAccessControl: + return view.user_access_control - if scope_object == "INTERNAL": - raise PermissionDenied(f"This action does not support Personal API Key access") + def _get_required_access_level(self, request, view) -> Optional[AccessControlLevel]: + resource = self._get_scope_object(request, view) + required_scopes = self._get_required_scopes(request, view) - action = self._get_action(request, view) - read_actions = getattr(view, "scope_object_read_actions", self.read_actions) - write_actions = getattr(view, "scope_object_write_actions", self.write_actions) + if resource == "INTERNAL": + return None - if action in write_actions: - return [f"{scope_object}:write"] - elif action in read_actions or request.method == "OPTIONS": - return [f"{scope_object}:read"] + READ_LEVEL = ordered_access_levels(resource)[-2] + WRITE_LEVEL = ordered_access_levels(resource)[-1] - # If we get here this typically means an action was called without a required scope - # It is essentially "INTERNAL" - raise PermissionDenied(f"This action does not support Personal API Key access") + if not required_scopes: + return READ_LEVEL if request.method in SAFE_METHODS else WRITE_LEVEL - def get_scope_object(self, request, view) -> APIScopeObjectOrNotSupported: - if not getattr(view, "scope_object", None): - raise ImproperlyConfigured("APIScopePermission requires the view to define the scope_object attribute.") + # TODO: This is definitely not right - we need to more safely map the scopes to access levels relevant to the object + for scope in required_scopes: + if scope.endswith(":write"): + return WRITE_LEVEL - return view.scope_object + return READ_LEVEL + + def has_object_permission(self, request, view, object) -> bool: + # At this level we are checking an individual resource - this could be a project or a lower level item like a Dashboard + + # NOTE: If the object is a Team then we shortcircuit here and create a UAC + # Reason being that there is a loop from view.user_access_control -> view.team -> view.user_access_control + if isinstance(object, Team): + uac = UserAccessControl(user=request.user, team=object) + else: + uac = self._get_user_access_control(request, view) + + if not uac: + # If the view doesn't have a user_access_control then it is not supported by this permission scheme + return True + + required_level = self._get_required_access_level(request, view) + + if not required_level: + return True + + has_access = uac.check_access_level_for_object(object, required_level=required_level) + + if not has_access: + self.message = f"You do not have {required_level} access to this resource." + return False + + return True + + def has_permission(self, request, view) -> bool: + # At this level we are checking that the user can generically access the resource kind. + # Primarily we are checking the user's access to the parent resource type (i.e. project, organization) + # as well as enforcing any global restrictions (e.g. generically only editing of a flag is allowed) + + uac = self._get_user_access_control(request, view) + scope_object = self._get_scope_object(request, view) + required_level = self._get_required_access_level(request, view) + + team: Team + + try: + team = view.team + except (ValueError, KeyError): + # TODO: Change this to a super specific exception... + # TODO: Does this means its okay because there is no team level thing? + return True + + # NOTE: This isn't perfect as it will only optimize for endpoints where the pk matches the obj.id + # We can't load the actual object as get_object in turn calls the permissions check + pk = view.kwargs.get("pk") + uac.preload_access_levels(team=team, resource=cast(APIScopeObject, scope_object), resource_id=pk) + + is_member = uac.check_access_level_for_object(team, required_level="member") + + if not is_member: + self.message = f"You don't have access to the project." + return False + + # If the API doesn't have a scope object or a required level for accessing then we can simply allow access + # as it isn't under access control + if scope_object == "INTERNAL" or not required_level: + return True + + # TODO: Scope object should probably be applied against the `required_scopes` attribute + has_access = uac.check_access_level_for_resource(scope_object, required_level=required_level) + + if not has_access: + self.message = f"You do not have {required_level} access to this resource." + return False + + return True class PostHogFeatureFlagPermission(BasePermission): diff --git a/posthog/rbac/access_control_api_mixin.py b/posthog/rbac/access_control_api_mixin.py new file mode 100644 index 0000000000000..ec684d56b7238 --- /dev/null +++ b/posthog/rbac/access_control_api_mixin.py @@ -0,0 +1,13 @@ +from typing import TYPE_CHECKING + + +if TYPE_CHECKING: + from ee.api.rbac.access_control import AccessControlViewSetMixin +else: + try: + from ee.api.rbac.access_control import AccessControlViewSetMixin + + except ImportError: + + class AccessControlViewSetMixin: + pass diff --git a/posthog/rbac/test/test_user_access_control.py b/posthog/rbac/test/test_user_access_control.py new file mode 100644 index 0000000000000..d30bc8d08685d --- /dev/null +++ b/posthog/rbac/test/test_user_access_control.py @@ -0,0 +1,538 @@ +import pytest +from posthog.constants import AvailableFeature +from posthog.models.dashboard import Dashboard +from posthog.models.organization import OrganizationMembership +from posthog.models.team.team import Team +from posthog.models.user import User +from posthog.rbac.user_access_control import UserAccessControl +from posthog.test.base import BaseTest + + +try: + from ee.models.rbac.access_control import AccessControl + from ee.models.rbac.role import Role, RoleMembership +except ImportError: + pass + + +class BaseUserAccessControlTest(BaseTest): + user_access_control: UserAccessControl + + def _create_access_control( + self, resource="project", resource_id=None, access_level="admin", organization_member=None, team=None, role=None + ): + ac, _ = AccessControl.objects.get_or_create( + team=self.team, + resource=resource, + resource_id=resource_id or self.team.id, + organization_member=organization_member, + role=role, + ) + + ac.access_level = access_level + ac.save() + + return ac + + def setUp(self): + super().setUp() + self.organization.available_features = [ + AvailableFeature.PROJECT_BASED_PERMISSIONING, + AvailableFeature.ROLE_BASED_ACCESS, + ] + self.organization.save() + + self.role_a = Role.objects.create(name="Engineers", organization=self.organization) + self.role_b = Role.objects.create(name="Administrators", organization=self.organization) + + RoleMembership.objects.create(user=self.user, role=self.role_a) + self.user_access_control = UserAccessControl(self.user, self.team) + + self.other_user = User.objects.create_and_join(self.organization, "other@posthog.com", "testtest") + RoleMembership.objects.create(user=self.other_user, role=self.role_b) + self.other_user_access_control = UserAccessControl(self.other_user, self.team) + + self.user_with_no_role = User.objects.create_and_join(self.organization, "norole@posthog.com", "testtest") + self.user_with_no_role_access_control = UserAccessControl(self.user_with_no_role, self.team) + + def _clear_uac_caches(self): + self.user_access_control._clear_cache() + self.other_user_access_control._clear_cache() + self.user_with_no_role_access_control._clear_cache() + + +@pytest.mark.ee +class TestUserAccessControl(BaseUserAccessControlTest): + def test_without_available_features(self): + self.organization.available_features = [] + self.organization.save() + self.organization_membership.level = OrganizationMembership.Level.ADMIN + self.organization_membership.save() + + assert self.user_access_control.access_level_for_object(self.team) == "admin" + assert self.user_access_control.check_access_level_for_object(self.team, "admin") is True + assert self.other_user_access_control.access_level_for_object(self.team) == "admin" + assert self.other_user_access_control.check_access_level_for_object(self.team, "admin") is True + assert self.user_access_control.access_level_for_resource("project") == "admin" + assert self.other_user_access_control.access_level_for_resource("project") == "admin" + assert self.user_access_control.check_can_modify_access_levels_for_object(self.team) is True + assert self.other_user_access_control.check_can_modify_access_levels_for_object(self.team) is False + + def test_ac_object_default_response(self): + self.organization_membership.level = OrganizationMembership.Level.ADMIN + self.organization_membership.save() + + assert self.user_access_control.access_level_for_object(self.team) == "admin" + assert self.user_access_control.check_access_level_for_object(self.team, "admin") is True + assert self.other_user_access_control.access_level_for_object(self.team) == "admin" + assert self.other_user_access_control.check_access_level_for_object(self.team, "admin") is True + assert self.user_access_control.access_level_for_resource("project") == "admin" + assert self.other_user_access_control.access_level_for_resource("project") == "admin" + assert self.user_access_control.check_can_modify_access_levels_for_object(self.team) is True + assert self.other_user_access_control.check_can_modify_access_levels_for_object(self.team) is False + + def test_ac_object_user_access_control(self): + # Setup member access by default + self._create_access_control(resource_id=self.team.id, access_level="member") + ac = self._create_access_control( + resource="project", + resource_id=str(self.team.id), + access_level="admin", + # context + organization_member=self.organization_membership, + ) + + assert self.user_access_control.access_level_for_object(self.team) == "admin" + assert self.user_access_control.check_access_level_for_object(self.team, "admin") is True + assert self.other_user_access_control.check_access_level_for_object(self.team, "admin") is False + + ac.access_level = "member" + ac.save() + self._clear_uac_caches() + + assert self.user_access_control.check_access_level_for_object(self.team, "admin") is False + assert self.user_access_control.check_access_level_for_object(self.team, "member") is True + assert ( + self.other_user_access_control.check_access_level_for_object(self.team, "member") + is True # This is the default + ) # Fix this - need to load all access controls... + + def test_ac_object_project_access_control(self): + # Setup no access by default + ac = self._create_access_control(resource_id=self.team.id, access_level="none") + + assert self.user_access_control.access_level_for_object(self.team) == "none" + assert self.user_access_control.check_access_level_for_object(self.team, "admin") is False + assert self.other_user_access_control.check_access_level_for_object(self.team, "admin") is False + + ac.access_level = "member" + ac.save() + self._clear_uac_caches() + + assert self.user_access_control.check_access_level_for_object(self.team, "admin") is False + assert self.user_access_control.check_access_level_for_object(self.team, "member") is True + assert self.other_user_access_control.check_access_level_for_object(self.team, "admin") is False + assert self.other_user_access_control.check_access_level_for_object(self.team, "member") is True + + ac.access_level = "admin" + ac.save() + self._clear_uac_caches() + + assert self.user_access_control.check_access_level_for_object(self.team, "admin") is True + assert self.other_user_access_control.check_access_level_for_object(self.team, "admin") is True + + def test_ac_object_role_access_control(self): + # Setup member access by default + self._create_access_control(resource_id=self.team.id, access_level="member") + ac = self._create_access_control(resource_id=self.team.id, access_level="admin", role=self.role_a) + + assert self.user_access_control.access_level_for_object(self.team) == "admin" + assert self.user_access_control.check_access_level_for_object(self.team, "admin") is True + assert self.other_user_access_control.check_access_level_for_object(self.team, "admin") is False + assert self.user_with_no_role_access_control.check_access_level_for_object(self.team, "admin") is False + + ac.access_level = "member" + ac.save() + self._clear_uac_caches() + + # Make the default access level none + self._create_access_control(resource_id=self.team.id, access_level="none") + + assert self.user_access_control.check_access_level_for_object(self.team, "admin") is False + assert self.user_access_control.check_access_level_for_object(self.team, "member") is True + assert self.other_user_access_control.check_access_level_for_object(self.team, "admin") is False + assert self.other_user_access_control.check_access_level_for_object(self.team, "member") is False + assert self.user_with_no_role_access_control.check_access_level_for_object(self.team, "admin") is False + + def test_ac_object_mixed_access_controls(self): + # No access by default + ac_project = self._create_access_control(resource_id=self.team.id, access_level="none") + # Enroll self.user as member + ac_user = self._create_access_control( + resource_id=self.team.id, access_level="member", organization_member=self.organization_membership + ) + # Enroll role_a as admin + ac_role = self._create_access_control( + resource_id=self.team.id, access_level="admin", role=self.role_a + ) # The highest AC + # Enroll role_b as member + ac_role_2 = self._create_access_control(resource_id=self.team.id, access_level="member", role=self.role_b) + # Enroll self.user in both roles + RoleMembership.objects.create(user=self.user, role=self.role_b) + + # Create an unrelated access control for self.user + self._create_access_control( + resource_id="something else", access_level="admin", organization_member=self.organization_membership + ) + + matching_acs = self.user_access_control._get_access_controls( + self.user_access_control._access_controls_filters_for_object("project", str(self.team.id)) + ) + assert len(matching_acs) == 4 + assert ac_project in matching_acs + assert ac_user in matching_acs + assert ac_role in matching_acs + assert ac_role_2 in matching_acs + # the matching one should be the highest level + assert self.user_access_control.access_level_for_object(self.team) == "admin" + + def test_org_admin_always_has_access(self): + self._create_access_control(resource_id=self.team.id, access_level="none") + assert self.other_user_access_control.check_access_level_for_object(self.team, "member") is False + assert self.other_user_access_control.check_access_level_for_object(self.team, "admin") is False + + self.organization_membership.level = OrganizationMembership.Level.ADMIN + self.organization_membership.save() + + assert self.user_access_control.check_access_level_for_object(self.team, "member") is True + assert self.user_access_control.check_access_level_for_object(self.team, "admin") is True + + def test_leaving_the_org_revokes_access(self): + self.user.leave(organization=self.organization) + assert self.user_access_control.check_access_level_for_object(self.team, "member") is False + + def test_filters_project_queryset_based_on_acs(self): + team2 = Team.objects.create(organization=self.organization) + team3 = Team.objects.create(organization=self.organization) + # No default access + self._create_access_control(resource="project", resource_id=team2.id, access_level="none") + # No default access + self._create_access_control(resource="project", resource_id=team3.id, access_level="none") + # This user access + self._create_access_control( + resource="project", + resource_id=team3.id, + access_level="member", + organization_member=self.organization_membership, + ) + + # NOTE: This is different to the API queries as the TeamAndOrgViewsetMixing takes care of filtering out based on the parent org + filtered_teams = list(self.user_access_control.filter_queryset_by_access_level(Team.objects.all())) + assert filtered_teams == [self.team, team3] + + other_user_filtered_teams = list( + self.other_user_access_control.filter_queryset_by_access_level(Team.objects.all()) + ) + assert other_user_filtered_teams == [self.team] + + def test_filters_project_queryset_based_on_acs_always_allows_org_admin(self): + team2 = Team.objects.create(organization=self.organization) + team3 = Team.objects.create(organization=self.organization) + # No default access + self._create_access_control(resource="project", resource_id=team2.id, access_level="none") + self._create_access_control(resource="project", resource_id=team3.id, access_level="none") + + self.organization_membership.level = OrganizationMembership.Level.ADMIN + self.organization_membership.save() + + filtered_teams = list( + self.user_access_control.filter_queryset_by_access_level(Team.objects.all(), include_all_if_admin=True) + ) + assert filtered_teams == [self.team, team2, team3] + + def test_organization_access_control(self): + # A team isn't always available like for organization level routing + + self.organization_membership.level = OrganizationMembership.Level.MEMBER + self.organization_membership.save() + + uac = UserAccessControl(user=self.user, organization_id=self.organization.id) + + assert uac.check_access_level_for_object(self.organization, "member") is True + assert uac.check_access_level_for_object(self.organization, "admin") is False + + self.organization_membership.level = OrganizationMembership.Level.ADMIN + self.organization_membership.save() + + uac = UserAccessControl(user=self.user, organization_id=self.organization.id) + + assert uac.check_access_level_for_object(self.organization, "admin") is True + + +class TestUserAccessControlResourceSpecific(BaseUserAccessControlTest): + """ + Most things are identical between "project"s and other resources, but there are some differences particularly in level names + """ + + def setUp(self): + super().setUp() + + self.dashboard = Dashboard.objects.create(team=self.team) + + def test_without_available_features(self): + self.organization.available_features = [] + self.organization.save() + self.organization_membership.level = OrganizationMembership.Level.ADMIN + self.organization_membership.save() + + assert self.user_access_control.access_level_for_object(self.dashboard) == "editor" + assert self.other_user_access_control.access_level_for_object(self.dashboard) == "editor" + assert self.user_access_control.access_level_for_resource("dashboard") == "editor" + assert self.other_user_access_control.access_level_for_resource("dashboard") == "editor" + + def test_ac_object_default_response(self): + assert self.user_access_control.access_level_for_object(self.dashboard) == "editor" + assert self.other_user_access_control.access_level_for_object(self.dashboard) == "editor" + + +# class TestUserDashboardPermissions(BaseTest, WithPermissionsBase): +# def setUp(self): +# super().setUp() +# self.organization.available_features = [AvailableFeature.ADVANCED_PERMISSIONS] +# self.organization.save() +# self.dashboard = Dashboard.objects.create(team=self.team) + +# def dashboard_permissions(self): +# return self.permissions().dashboard(self.dashboard) + +# def test_dashboard_effective_restriction_level(self): +# assert ( +# self.dashboard_permissions().effective_restriction_level +# == Dashboard.RestrictionLevel.EVERYONE_IN_PROJECT_CAN_EDIT +# ) + +# def test_dashboard_effective_restriction_level_explicit(self): +# self.dashboard.restriction_level = Dashboard.RestrictionLevel.ONLY_COLLABORATORS_CAN_EDIT +# self.dashboard.save() + +# assert ( +# self.dashboard_permissions().effective_restriction_level +# == Dashboard.RestrictionLevel.ONLY_COLLABORATORS_CAN_EDIT +# ) + +# def test_dashboard_effective_restriction_level_when_feature_not_available(self): +# self.organization.available_features = [] +# self.organization.save() + +# self.dashboard.restriction_level = Dashboard.RestrictionLevel.ONLY_COLLABORATORS_CAN_EDIT +# self.dashboard.save() + +# assert ( +# self.dashboard_permissions().effective_restriction_level +# == Dashboard.RestrictionLevel.EVERYONE_IN_PROJECT_CAN_EDIT +# ) + +# def test_dashboard_can_restrict(self): +# assert not self.dashboard_permissions().can_restrict + +# def test_dashboard_can_restrict_as_admin(self): +# self.organization_membership.level = OrganizationMembership.Level.ADMIN +# self.organization_membership.save() + +# assert self.dashboard_permissions().can_restrict + +# def test_dashboard_can_restrict_as_creator(self): +# self.dashboard.created_by = self.user +# self.dashboard.save() + +# assert self.dashboard_permissions().can_restrict + +# def test_dashboard_effective_privilege_level_when_everyone_can_edit(self): +# self.dashboard.restriction_level = Dashboard.RestrictionLevel.EVERYONE_IN_PROJECT_CAN_EDIT +# self.dashboard.save() + +# assert self.dashboard_permissions().effective_privilege_level == Dashboard.PrivilegeLevel.CAN_EDIT + +# def test_dashboard_effective_privilege_level_when_collaborators_can_edit(self): +# self.dashboard.restriction_level = Dashboard.RestrictionLevel.ONLY_COLLABORATORS_CAN_EDIT +# self.dashboard.save() + +# assert self.dashboard_permissions().effective_privilege_level == Dashboard.PrivilegeLevel.CAN_VIEW + +# def test_dashboard_effective_privilege_level_priviledged(self): +# self.dashboard.restriction_level = Dashboard.RestrictionLevel.ONLY_COLLABORATORS_CAN_EDIT +# self.dashboard.save() + +# DashboardPrivilege.objects.create( +# user=self.user, +# dashboard=self.dashboard, +# level=Dashboard.PrivilegeLevel.CAN_EDIT, +# ) + +# assert self.dashboard_permissions().effective_privilege_level == Dashboard.PrivilegeLevel.CAN_EDIT + +# def test_dashboard_effective_privilege_level_creator(self): +# self.dashboard.restriction_level = Dashboard.RestrictionLevel.ONLY_COLLABORATORS_CAN_EDIT +# self.dashboard.save() +# self.dashboard.created_by = self.user +# self.dashboard.save() + +# assert self.dashboard_permissions().effective_privilege_level == Dashboard.PrivilegeLevel.CAN_EDIT + +# def test_dashboard_can_edit_when_everyone_can(self): +# self.dashboard.restriction_level = Dashboard.RestrictionLevel.EVERYONE_IN_PROJECT_CAN_EDIT +# self.dashboard.save() + +# assert self.dashboard_permissions().can_edit + +# def test_dashboard_can_edit_not_collaborator(self): +# self.dashboard.restriction_level = Dashboard.RestrictionLevel.ONLY_COLLABORATORS_CAN_EDIT +# self.dashboard.save() + +# assert not self.dashboard_permissions().can_edit + +# def test_dashboard_can_edit_creator(self): +# self.dashboard.restriction_level = Dashboard.RestrictionLevel.ONLY_COLLABORATORS_CAN_EDIT +# self.dashboard.save() +# self.dashboard.created_by = self.user +# self.dashboard.save() + +# assert self.dashboard_permissions().can_edit + +# def test_dashboard_can_edit_priviledged(self): +# self.dashboard.restriction_level = Dashboard.RestrictionLevel.ONLY_COLLABORATORS_CAN_EDIT +# self.dashboard.save() + +# DashboardPrivilege.objects.create( +# user=self.user, +# dashboard=self.dashboard, +# level=Dashboard.PrivilegeLevel.CAN_EDIT, +# ) + +# assert self.dashboard_permissions().can_edit + + +# class TestUserInsightPermissions(BaseTest, WithPermissionsBase): +# def setUp(self): +# super().setUp() +# self.organization.available_features = [AvailableFeature.ADVANCED_PERMISSIONS] +# self.organization.save() + +# self.dashboard1 = Dashboard.objects.create( +# team=self.team, +# restriction_level=Dashboard.RestrictionLevel.ONLY_COLLABORATORS_CAN_EDIT, +# ) +# self.dashboard2 = Dashboard.objects.create(team=self.team) +# self.insight = Insight.objects.create(team=self.team) +# self.tile1 = DashboardTile.objects.create(dashboard=self.dashboard1, insight=self.insight) +# self.tile2 = DashboardTile.objects.create(dashboard=self.dashboard2, insight=self.insight) + +# def insight_permissions(self): +# return self.permissions().insight(self.insight) + +# def test_effective_restriction_level_limited(self): +# assert ( +# self.insight_permissions().effective_restriction_level +# == Dashboard.RestrictionLevel.ONLY_COLLABORATORS_CAN_EDIT +# ) + +# def test_effective_restriction_level_all_allow(self): +# Dashboard.objects.all().update(restriction_level=Dashboard.RestrictionLevel.EVERYONE_IN_PROJECT_CAN_EDIT) + +# assert ( +# self.insight_permissions().effective_restriction_level +# == Dashboard.RestrictionLevel.EVERYONE_IN_PROJECT_CAN_EDIT +# ) + +# def test_effective_restriction_level_with_no_dashboards(self): +# DashboardTile.objects.all().delete() + +# assert ( +# self.insight_permissions().effective_restriction_level +# == Dashboard.RestrictionLevel.EVERYONE_IN_PROJECT_CAN_EDIT +# ) + +# def test_effective_restriction_level_with_no_permissioning(self): +# self.organization.available_features = [] +# self.organization.save() + +# assert ( +# self.insight_permissions().effective_restriction_level +# == Dashboard.RestrictionLevel.EVERYONE_IN_PROJECT_CAN_EDIT +# ) + +# def test_effective_privilege_level_all_limited(self): +# Dashboard.objects.all().update(restriction_level=Dashboard.RestrictionLevel.ONLY_COLLABORATORS_CAN_EDIT) + +# assert self.insight_permissions().effective_privilege_level == Dashboard.PrivilegeLevel.CAN_VIEW + +# def test_effective_privilege_level_some_limited(self): +# assert self.insight_permissions().effective_privilege_level == Dashboard.PrivilegeLevel.CAN_EDIT + +# def test_effective_privilege_level_all_limited_as_collaborator(self): +# Dashboard.objects.all().update(restriction_level=Dashboard.RestrictionLevel.ONLY_COLLABORATORS_CAN_EDIT) +# self.dashboard1.created_by = self.user +# self.dashboard1.save() + +# assert self.insight_permissions().effective_privilege_level == Dashboard.PrivilegeLevel.CAN_EDIT + +# def test_effective_privilege_level_with_no_dashboards(self): +# DashboardTile.objects.all().delete() + +# assert self.insight_permissions().effective_privilege_level == Dashboard.PrivilegeLevel.CAN_EDIT + + +# class TestUserPermissionsEfficiency(BaseTest, WithPermissionsBase): +# def test_dashboard_efficiency(self): +# self.organization.available_features = [ +# AvailableFeature.PROJECT_BASED_PERMISSIONING, +# AvailableFeature.ADVANCED_PERMISSIONS, +# ] +# self.organization.save() + +# dashboard = Dashboard.objects.create( +# team=self.team, +# restriction_level=Dashboard.RestrictionLevel.ONLY_COLLABORATORS_CAN_EDIT, +# ) +# insights, tiles = [], [] +# for _ in range(10): +# insight = Insight.objects.create(team=self.team) +# tile = DashboardTile.objects.create(dashboard=dashboard, insight=insight) +# insights.append(insight) +# tiles.append(tile) + +# user_permissions = self.permissions() +# user_permissions.set_preloaded_dashboard_tiles(tiles) + +# with self.assertNumQueries(3): +# assert user_permissions.current_team.effective_membership_level is not None +# assert user_permissions.dashboard(dashboard).effective_restriction_level is not None +# assert user_permissions.dashboard(dashboard).can_restrict is not None +# assert user_permissions.dashboard(dashboard).effective_privilege_level is not None +# assert user_permissions.dashboard(dashboard).can_edit is not None + +# for insight in insights: +# assert user_permissions.insight(insight).effective_restriction_level is not None +# assert user_permissions.insight(insight).effective_privilege_level is not None + +# def test_team_lookup_efficiency(self): +# user = User.objects.create(email="test2@posthog.com", distinct_id="test2") +# models = [] +# for _ in range(10): +# organization, membership, team = Organization.objects.bootstrap( +# user=user, team_fields={"access_control": True} +# ) +# membership.level = OrganizationMembership.Level.ADMIN # type: ignore +# membership.save() # type: ignore + +# organization.available_features = [AvailableFeature.PROJECT_BASED_PERMISSIONING] +# organization.save() + +# models.append((organization, membership, team)) + +# user_permissions = UserPermissions(user) +# with self.assertNumQueries(3): +# assert len(user_permissions.team_ids_visible_for_user) == 10 + +# for _, _, team in models: +# assert user_permissions.team(team).effective_membership_level == OrganizationMembership.Level.ADMIN diff --git a/posthog/rbac/user_access_control.py b/posthog/rbac/user_access_control.py new file mode 100644 index 0000000000000..477e0669ef03e --- /dev/null +++ b/posthog/rbac/user_access_control.py @@ -0,0 +1,486 @@ +from functools import cached_property +import json +from django.contrib.auth.models import AnonymousUser +from django.db.models import Model, Q, QuerySet +from rest_framework import serializers +from typing import TYPE_CHECKING, Any, Literal, Optional, cast, get_args + +from posthog.constants import AvailableFeature +from posthog.models import ( + Organization, + OrganizationMembership, + Team, + User, +) +from posthog.models.scopes import APIScopeObject, API_SCOPE_OBJECTS + + +if TYPE_CHECKING: + from ee.models import AccessControl + + _AccessControl = AccessControl +else: + _AccessControl = object + + +try: + from ee.models.rbac.access_control import AccessControl +except ImportError: + pass + +AccessControlLevelNone = Literal["none"] +AccessControlLevelMember = Literal[AccessControlLevelNone, "member", "admin"] +AccessControlLevelResource = Literal[AccessControlLevelNone, "viewer", "editor"] +AccessControlLevel = Literal[AccessControlLevelMember, AccessControlLevelResource] + +NO_ACCESS_LEVEL = "none" +ACCESS_CONTROL_LEVELS_MEMBER: tuple[AccessControlLevelMember, ...] = get_args(AccessControlLevelMember) +ACCESS_CONTROL_LEVELS_RESOURCE: tuple[AccessControlLevelResource, ...] = get_args(AccessControlLevelResource) + + +def ordered_access_levels(resource: APIScopeObject) -> list[AccessControlLevel]: + if resource in ["project", "organization"]: + return list(ACCESS_CONTROL_LEVELS_MEMBER) + + return list(ACCESS_CONTROL_LEVELS_RESOURCE) + + +def default_access_level(resource: APIScopeObject) -> AccessControlLevel: + if resource in ["project"]: + return "admin" + if resource in ["organization"]: + return "member" + return "editor" + + +def highest_access_level(resource: APIScopeObject) -> AccessControlLevel: + return ordered_access_levels(resource)[-1] + + +def access_level_satisfied_for_resource( + resource: APIScopeObject, current_level: AccessControlLevel, required_level: AccessControlLevel +) -> bool: + return ordered_access_levels(resource).index(current_level) >= ordered_access_levels(resource).index(required_level) + + +def model_to_resource(model: Model) -> Optional[APIScopeObject]: + """ + Given a model, return the resource type it represents + """ + if hasattr(model, "_meta"): + name = model._meta.model_name + else: + name = model.__class__.__name__.lower() + + # NOTE: These are special mappings where the 1-1 of APIScopeObject doesn't match + if name == "team": + return "project" + if name == "featureflag": + return "feature_flag" + if name == "plugin_config": + return "plugin" + + if name not in API_SCOPE_OBJECTS: + return None + + return cast(APIScopeObject, name) + + +class UserAccessControl: + """ + UserAccessControl provides functions for checking unified access to all resources and objects from a Project level downwards. + Typically a Team (Project) is required other than in certain circumstances, particularly when validating which projects a user has access to within an organization. + """ + + def __init__(self, user: User, team: Optional[Team] = None, organization_id: Optional[str] = None): + self._user = user + self._team = team + + if not organization_id and team: + organization_id = str(team.organization_id) + + if not organization_id: + raise ValueError("Organization ID must be provided either directly or via the team") + + self._organization_id = organization_id + self._cache: dict[str, list[AccessControl]] = {} + + def _clear_cache(self): + # Primarily intended for tests + self._cache = {} + + @cached_property + def _organization_membership(self) -> Optional[OrganizationMembership]: + # NOTE: This is optimized to reduce queries - we get the users membership _with_ the organization + try: + return OrganizationMembership.objects.select_related("organization").get( + organization_id=self._organization_id, user=self._user + ) + except OrganizationMembership.DoesNotExist: + return None + + @cached_property + def _organization(self) -> Optional[Organization]: + if self._organization_membership: + return self._organization_membership.organization + return None + + @cached_property + def _user_role_ids(self): + if not self.rbac_supported: + # Early return to prevent an unnecessary lookup + return [] + + role_memberships = cast(Any, self._user).role_memberships.select_related("role").all() + return [membership.role.id for membership in role_memberships] + + @property + def rbac_supported(self) -> bool: + if not self._organization: + return False + + return self._organization.is_feature_available(AvailableFeature.ROLE_BASED_ACCESS) + + @property + def access_controls_supported(self) -> bool: + # NOTE: This is a proxy feature. We may want to consider making it explicit later + # ADVANCED_PERMISSIONS was only for dashboard collaborators, PROJECT_BASED_PERMISSIONING for project permissions + # both now apply to this generic access control + + if not self._organization: + return False + + return self._organization.is_feature_available( + AvailableFeature.PROJECT_BASED_PERMISSIONING + ) or self._organization.is_feature_available(AvailableFeature.ADVANCED_PERMISSIONS) + + def _filter_options(self, filters: dict[str, Any]) -> Q: + """ + Adds the 3 main filter options to the query + """ + return ( + Q( # Access controls applying to this team + **filters, organization_member=None, role=None + ) + | Q( # Access controls applying to this user + **filters, organization_member__user=self._user, role=None + ) + | Q( # Access controls applying to this user's roles + **filters, organization_member=None, role__in=self._user_role_ids + ) + ) + + def _get_access_controls(self, filters: dict) -> list[_AccessControl]: + key = json.dumps(filters, sort_keys=True) + if key not in self._cache: + self._cache[key] = list(AccessControl.objects.filter(self._filter_options(filters))) + + return self._cache[key] + + def _access_controls_filters_for_object(self, resource: APIScopeObject, resource_id: str) -> dict: + """ + Used when checking an individual object - gets all access controls for the object and its type + """ + return {"team_id": self._team.id, "resource": resource, "resource_id": resource_id} # type: ignore + + def _access_controls_filters_for_resource(self, resource: APIScopeObject) -> dict: + """ + Used when checking overall access to a resource + """ + + return {"team_id": self._team.id, "resource": resource, "resource_id": None} # type: ignore + + def _access_controls_filters_for_queryset(self, resource: APIScopeObject) -> dict: + """ + Used to filter out IDs from a queryset based on access controls where the specific resource is denied access + """ + common_filters: dict[str, Any] = {"resource": resource, "resource_id__isnull": False} + + if self._team and resource != "project": + common_filters["team_id"] = self._team.id + else: + common_filters["team__organization_id"] = str(self._organization_id) + + return common_filters + + def _fill_filters_cache(self, filter_groups: list[dict], access_controls: list[_AccessControl]) -> None: + for filters in filter_groups: + key = json.dumps(filters, sort_keys=True) + + # TRICKY: We have to simulate the entire DB query here: + matching_access_controls = [] + + for ac in access_controls: + matches = True + for key, value in filters.items(): + if key == "resource_id__isnull": + if (ac.resource_id is None) != value: + matches = False + break + elif key == "team__organization_id": + if ac.team.organization_id != value: + matches = False + break + elif getattr(ac, key) != value: + matches = False + break + if matches: + matching_access_controls.append(ac) + + self._cache[key] = matching_access_controls + + def preload_object_access_controls(self, objects: list[Model]) -> None: + """ + Preload access controls for a list of objects + """ + + filter_groups: list[dict] = [] + + for obj in objects: + resource = model_to_resource(obj) + if not resource: + return + + filter_groups.append(self._access_controls_filters_for_object(resource, str(obj.id))) # type: ignore + + q = Q() + for filters in filter_groups: + q = q | self._filter_options(filters) + + access_controls = list(AccessControl.objects.filter(q)) + self._fill_filters_cache(filter_groups, access_controls) + + def preload_access_levels(self, team: Team, resource: APIScopeObject, resource_id: Optional[str] = None) -> None: + """ + Checking permissions can involve multiple queries to AccessControl e.g. project level, global resource level, and object level + As we can know this upfront, we can optimize this by loading all the controls we will need upfront. + """ + # Question - are we fundamentally loading every access control for the given resource? If so should we accept that fact and just load them all? + # doing all additional filtering in memory? + + filter_groups: list[dict] = [] + + filter_groups.append(self._access_controls_filters_for_object(resource="project", resource_id=str(team.id))) + filter_groups.append(self._access_controls_filters_for_resource(resource)) + + if resource_id: + filter_groups.append(self._access_controls_filters_for_object(resource, resource_id=resource_id)) + else: + filter_groups.append(self._access_controls_filters_for_queryset(resource)) + + q = Q() + for filters in filter_groups: + q = q | self._filter_options(filters) + + access_controls = list(AccessControl.objects.filter(q)) + self._fill_filters_cache(filter_groups, access_controls) + + # Object level - checking conditions for specific items + def access_level_for_object( + self, obj: Model, resource: Optional[APIScopeObject] = None, explicit=False + ) -> Optional[AccessControlLevel]: + """ + Access levels are strings - the order of which is determined at run time. + We find all relevant access controls and then return the highest value + """ + + resource = resource or model_to_resource(obj) + org_membership = self._organization_membership + + if not resource or not org_membership: + return None + + # Creators always have highest access + if getattr(obj, "created_by", None) == self._user: + return highest_access_level(resource) + + # Org admins always have highest access + if org_membership.level >= OrganizationMembership.Level.ADMIN: + return highest_access_level(resource) + + if resource == "organization": + # Organization access is controlled via membership level only + if org_membership.level >= OrganizationMembership.Level.ADMIN: + return "admin" + return "member" + + # If access controls aren't supported, then we return the default access level + if not self.access_controls_supported: + return default_access_level(resource) if not explicit else None + + filters = self._access_controls_filters_for_object(resource, str(obj.id)) # type: ignore + access_controls = self._get_access_controls(filters) + + # If there is no specified controls on the resource then we return the default access level + if not access_controls: + return default_access_level(resource) if not explicit else None + + # If there are access controls we pick the highest level the user has + return max( + access_controls, + key=lambda access_control: ordered_access_levels(resource).index(access_control.access_level), + ).access_level + + def check_access_level_for_object( + self, obj: Model, required_level: AccessControlLevel, explicit=False + ) -> Optional[bool]: + """ + Entry point for all permissions around a specific object. + If any of the access controls have the same or higher level than the requested level, return True. + + Returns true or false if access controls are applied, otherwise None + """ + + resource = model_to_resource(obj) + if not resource: + # Permissions do not apply to models without a related scope + return True + + access_level = self.access_level_for_object(obj, resource, explicit=explicit) + + if not access_level: + return False + + # If no access control exists + return access_level_satisfied_for_resource(resource, access_level, required_level) + + def check_can_modify_access_levels_for_object(self, obj: Model) -> Optional[bool]: + """ + Special case for checking if the user can modify the access levels for an object. + Unlike check_access_level_for_object, this requires that one of these conditions is true: + 1. The user is the creator of the object + 2. The user is explicitly a project admin + 2. The user is an org admin + """ + + if getattr(obj, "created_by", None) == self._user: + # TODO: Should this always be the case, even for projects? + return True + + # If they aren't the creator then they need to be a project admin or org admin + # TRICKY: If self._team isn't set, this is likely called for a Team itself so we pass in the object + return self.check_access_level_for_object(self._team or obj, required_level="admin", explicit=True) + + # Resource level - checking conditions for the resource type + def access_level_for_resource(self, resource: APIScopeObject) -> Optional[AccessControlLevel]: + """ + Access levels are strings - the order of which is determined at run time. + We find all relevant access controls and then return the highest value + """ + + org_membership = self._organization_membership + + if not resource or not org_membership: + # In any of these cases, we can't determine the access level + return None + + # Org admins always have resource level access + if org_membership.level >= OrganizationMembership.Level.ADMIN: + return highest_access_level(resource) + + if not self.access_controls_supported: + # If access controls aren't supported, then return the default access level + return default_access_level(resource) + + filters = self._access_controls_filters_for_resource(resource) + access_controls = self._get_access_controls(filters) + + if not access_controls: + return default_access_level(resource) + + return max( + access_controls, + key=lambda access_control: ordered_access_levels(resource).index(access_control.access_level), + ).access_level + + def check_access_level_for_resource(self, resource: APIScopeObject, required_level: AccessControlLevel) -> bool: + access_level = self.access_level_for_resource(resource) + + if not access_level: + return False + + return access_level_satisfied_for_resource(resource, access_level, required_level) + + def filter_queryset_by_access_level(self, queryset: QuerySet, include_all_if_admin=False) -> 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 + + model = cast(Model, queryset.model) + resource = model_to_resource(model) + + if not resource: + return queryset + + if include_all_if_admin: + org_membership = self._organization_membership + + if org_membership and org_membership.level >= OrganizationMembership.Level.ADMIN: + return queryset + + model_has_creator = hasattr(model, "created_by") + + filters = self._access_controls_filters_for_queryset(resource) + access_controls = self._get_access_controls(filters) + + blocked_resource_ids: set[str] = set() + resource_id_access_levels: dict[str, list[str]] = {} + + for access_control in access_controls: + resource_id_access_levels.setdefault(access_control.resource_id, []).append(access_control.access_level) + + for resource_id, access_levels in resource_id_access_levels.items(): + # Check if every access level is "none" + if all(access_level == NO_ACCESS_LEVEL for access_level in access_levels): + blocked_resource_ids.add(resource_id) + + # Filter the queryset based on the access controls + if 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 + + +class UserAccessControlSerializerMixin(serializers.Serializer): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self._preloaded_access_controls = False + + user_access_level = serializers.SerializerMethodField( + read_only=True, + help_text="The effective access level the user has for this object", + ) + + @cached_property + def user_access_control(self) -> Optional[UserAccessControl]: + # NOTE: The user_access_control is typically on the view but in specific cases such as the posthog_app_context it is set at the context level + if "user_access_control" in self.context: + # Get it directly from the context + return self.context["user_access_control"] + elif hasattr(self.context.get("view", None), "user_access_control"): + # Otherwise from the view (the default case) + return self.context["view"].user_access_control + else: + user = cast(User | AnonymousUser, self.context["request"].user) + # The user could be anonymous - if so there is no access control to be used + + if user.is_anonymous: + return None + + user = cast(User, user) + + return UserAccessControl(user, organization_id=str(user.current_organization_id)) + + def get_user_access_level(self, obj: Model) -> Optional[str]: + if not self.user_access_control: + return None + + # Check if self.instance is a list - if so we want to preload the user access controls + if not self._preloaded_access_controls and isinstance(self.instance, list): + self.user_access_control.preload_object_access_controls(self.instance) + self._preloaded_access_controls = True + + return self.user_access_control.access_level_for_object(obj) diff --git a/posthog/test/test_middleware.py b/posthog/test/test_middleware.py index 507c247d5b0c1..7a58996963dbf 100644 --- a/posthog/test/test_middleware.py +++ b/posthog/test/test_middleware.py @@ -142,7 +142,7 @@ class TestAutoProjectMiddleware(APIBaseTest): @classmethod def setUpTestData(cls): super().setUpTestData() - cls.base_app_num_queries = 45 + cls.base_app_num_queries = 47 # Create another team that the user does have access to cls.second_team = create_team(organization=cls.organization, name="Second Life") @@ -164,7 +164,7 @@ def setUp(self): def test_project_switched_when_accessing_dashboard_of_another_accessible_team(self): dashboard = Dashboard.objects.create(team=self.second_team) - with self.assertNumQueries(self.base_app_num_queries + 4): # AutoProjectMiddleware adds 4 queries + with self.assertNumQueries(self.base_app_num_queries + 5): # AutoProjectMiddleware adds 5 queries response_app = self.client.get(f"/dashboard/{dashboard.id}") response_users_api = self.client.get(f"/api/users/@me/") response_users_api_data = response_users_api.json() @@ -205,10 +205,10 @@ def test_project_unchanged_when_accessing_dashboard_of_another_off_limits_team(s self.user.refresh_from_db() response_dashboards_api = self.client.get(f"/api/projects/@current/dashboards/{dashboard.id}/") - self.assertEqual(response_app.status_code, 200) - self.assertEqual(response_users_api.status_code, 200) - self.assertEqual(response_users_api_data.get("team", {}).get("id"), self.team.id) # NOT third_team - self.assertEqual(response_dashboards_api.status_code, 404) + assert response_app.status_code == 200 + assert response_users_api.status_code == 200 + assert response_users_api_data.get("team", {}).get("id") == self.team.id # NOT third_team + assert response_dashboards_api.status_code == 404, response_dashboards_api.json() @override_settings(PERSON_ON_EVENTS_V2_OVERRIDE=False) def test_project_unchanged_when_accessing_dashboards_list(self): @@ -282,7 +282,7 @@ def test_project_switched_when_accessing_cohort_of_another_accessible_team(self) def test_project_switched_when_accessing_feature_flag_of_another_accessible_team(self): feature_flag = FeatureFlag.objects.create(team=self.second_team, created_by=self.user) - with self.assertNumQueries(self.base_app_num_queries + 4): + with self.assertNumQueries(self.base_app_num_queries + 5): response_app = self.client.get(f"/feature_flags/{feature_flag.id}") response_users_api = self.client.get(f"/api/users/@me/") response_users_api_data = response_users_api.json() diff --git a/posthog/utils.py b/posthog/utils.py index 7535df0700638..d8ea9315fec7c 100644 --- a/posthog/utils.py +++ b/posthog/utils.py @@ -368,6 +368,7 @@ def render_template( from posthog.api.project import ProjectSerializer from posthog.api.user import UserSerializer from posthog.user_permissions import UserPermissions + from posthog.rbac.user_access_control import UserAccessControl from posthog.views import preflight_check posthog_app_context = { @@ -390,9 +391,14 @@ def render_template( elif request.user.pk: user = cast("User", request.user) user_permissions = UserPermissions(user=user, team=user.team) + user_access_control = UserAccessControl(user=user, team=user.team) user_serialized = UserSerializer( request.user, - context={"request": request, "user_permissions": user_permissions}, + context={ + "request": request, + "user_permissions": user_permissions, + "user_access_control": user_access_control, + }, many=False, ) posthog_app_context["current_user"] = user_serialized.data @@ -400,7 +406,11 @@ def render_template( if user.team: team_serialized = TeamSerializer( user.team, - context={"request": request, "user_permissions": user_permissions}, + context={ + "request": request, + "user_permissions": user_permissions, + "user_access_control": user_access_control, + }, many=False, ) posthog_app_context["current_team"] = team_serialized.data