Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(projects): Fix project creation when access_control set #24988

Merged
merged 2 commits into from
Sep 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions ee/api/test/test_team.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@
HTTP_204_NO_CONTENT,
HTTP_403_FORBIDDEN,
HTTP_404_NOT_FOUND,
HTTP_400_BAD_REQUEST,
)

from ee.api.test.base import APILicensedTest
from ee.models.explicit_team_membership import ExplicitTeamMembership
from posthog.models.dashboard import Dashboard
from posthog.models.organization import Organization, OrganizationMembership
from posthog.models.project import Project
from posthog.models.team import Team
Expand Down Expand Up @@ -40,6 +42,26 @@ def test_create_team(self):
)
self.assertEqual(self.organization.teams.count(), 2)

def test_create_team_with_access_control(self):
self.organization_membership.level = OrganizationMembership.Level.ADMIN
self.organization_membership.save()
self.assertEqual(Team.objects.count(), 1)
self.assertEqual(Project.objects.count(), 1)
response = self.client.post("/api/environments/", {"name": "Test", "access_control": True})
self.assertEqual(response.status_code, 201)
self.assertEqual(Team.objects.count(), 2)
self.assertEqual(Project.objects.count(), 2)
response_data = response.json()
self.assertDictContainsSubset(
{
"name": "Test",
"access_control": True,
"effective_membership_level": OrganizationMembership.Level.ADMIN,
},
response_data,
)
self.assertEqual(self.organization.teams.count(), 2)

def test_non_admin_cannot_create_team(self):
self.organization_membership.level = OrganizationMembership.Level.MEMBER
self.organization_membership.save()
Expand All @@ -52,6 +74,19 @@ def test_non_admin_cannot_create_team(self):
self.permission_denied_response("Your organization access level is insufficient."),
)

def test_cannot_create_team_with_primary_dashboard_id(self):
dashboard_x = Dashboard.objects.create(team=self.team, name="Test")
self.organization_membership.level = OrganizationMembership.Level.ADMIN
self.organization_membership.save()
response = self.client.post("/api/environments/", {"name": "Test", "primary_dashboard": dashboard_x.id})
self.assertEqual(response.status_code, HTTP_400_BAD_REQUEST, response.json())
self.assertEqual(
response.json(),
self.validation_error_response(
"Primary dashboard cannot be set on project creation.", attr="primary_dashboard"
),
)

def test_create_demo_team(self, *args):
self.organization_membership.level = OrganizationMembership.Level.ADMIN
self.organization_membership.save()
Expand Down
2 changes: 2 additions & 0 deletions posthog/api/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@


class ProjectSerializer(ProjectBasicSerializer, UserPermissionsSerializerMixin):
instance: Optional[Project]

effective_membership_level = serializers.SerializerMethodField() # Compat with TeamSerializer
has_group_types = serializers.SerializerMethodField() # Compat with TeamSerializer
live_events_token = serializers.SerializerMethodField() # Compat with TeamSerializer
Expand Down
23 changes: 18 additions & 5 deletions posthog/api/team.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from functools import cached_property
from typing import Any, Optional, cast
from datetime import timedelta
from uuid import UUID

from django.shortcuts import get_object_or_404
from loginas.utils import is_impersonated_session
Expand All @@ -27,6 +28,7 @@
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.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
Expand Down Expand Up @@ -110,6 +112,8 @@ class Meta:


class TeamSerializer(serializers.ModelSerializer, UserPermissionsSerializerMixin):
instance: Optional[Team]

effective_membership_level = serializers.SerializerMethodField()
has_group_types = serializers.SerializerMethodField()
live_events_token = serializers.SerializerMethodField()
Expand Down Expand Up @@ -512,20 +516,29 @@ class RootTeamViewSet(TeamViewSet):


def validate_team_attrs(
attrs: dict[str, Any], view: TeamAndOrgViewSetMixin, request: request.Request, instance
attrs: dict[str, Any], view: TeamAndOrgViewSetMixin, request: request.Request, instance: Optional[Team | Project]
) -> dict[str, Any]:
if "primary_dashboard" in attrs and attrs["primary_dashboard"].team_id != instance.id:
raise exceptions.PermissionDenied("Dashboard does not belong to this team.")
if "primary_dashboard" in attrs:
if not instance:
raise exceptions.ValidationError(
{"primary_dashboard": "Primary dashboard cannot be set on project creation."}
)
if attrs["primary_dashboard"].team_id != instance.id:
raise exceptions.ValidationError({"primary_dashboard": "Dashboard does not belong to this team."})

if "access_control" in attrs:
assert isinstance(request.user, User)
# We get the instance's organization_id, unless we're handling creation, in which case there's no instance yet
organization_id = instance.organization_id if instance is not None else cast(UUID | str, view.organization_id)
# Only organization-wide admins and above should be allowed to switch the project between open and private
# If a project-only admin who is only an org member disabled this it, they wouldn't be able to reenable it
org_membership: OrganizationMembership = OrganizationMembership.objects.only("level").get(
organization_id=instance.organization_id, user=request.user
organization_id=organization_id, user=request.user
)
if org_membership.level < OrganizationMembership.Level.ADMIN:
raise exceptions.PermissionDenied("Your organization access level is insufficient.")
raise exceptions.PermissionDenied(
"Your organization access level is insufficient to configure project access restrictions."
)

if "autocapture_exceptions_errors_to_ignore" in attrs:
if not isinstance(attrs["autocapture_exceptions_errors_to_ignore"], list):
Expand Down
12 changes: 6 additions & 6 deletions posthog/api/test/test_team.py
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,7 @@ def test_update_primary_dashboard(self):
response = self.client.patch("/api/environments/@current/", {"primary_dashboard": d.id})
response_data = response.json()

self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(response.status_code, status.HTTP_200_OK, response.json())
self.assertEqual(response_data["name"], self.team.name)
self.assertEqual(response_data["primary_dashboard"], d.id)

Expand All @@ -550,11 +550,11 @@ def test_cant_set_primary_dashboard_to_another_teams_dashboard(self):
d = Dashboard.objects.create(name="Test", team=team_2)

response = self.client.patch("/api/environments/@current/", {"primary_dashboard": d.id})
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)

response = self.client.get("/api/environments/@current/")
response_data = response.json()
self.assertEqual(response_data["primary_dashboard"], None)
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertEqual(
response.json(),
self.validation_error_response("Dashboard does not belong to this team.", attr="primary_dashboard"),
)

def test_is_generating_demo_data(self):
cache_key = f"is_generating_demo_data_{self.team.pk}"
Expand Down
Loading