From 9d669cfbaad36ca3708bdfd61edd9297852f0a4e Mon Sep 17 00:00:00 2001 From: Evandro Myller Date: Wed, 11 Mar 2026 21:23:49 -0300 Subject: [PATCH 1/5] Fix ANY project sneaking in segment creation --- api/segments/serializers.py | 23 +++++++++++++++---- api/segments/views.py | 3 +++ .../unit/segments/test_unit_segments_views.py | 23 +++++++++++++++++++ 3 files changed, 45 insertions(+), 4 deletions(-) diff --git a/api/segments/serializers.py b/api/segments/serializers.py index 2bf44a4778e1..eba37d9ac4a6 100644 --- a/api/segments/serializers.py +++ b/api/segments/serializers.py @@ -1,3 +1,4 @@ +from functools import cached_property from typing import Any import structlog @@ -79,6 +80,7 @@ class Meta: class SegmentSerializer(MetadataSerializerMixin, WritableNestedModelSerializer): + instance: Segment | None rules = SegmentRuleSerializer(many=True, required=True, allow_empty=False) metadata = MetadataSerializer(required=False, many=True) @@ -112,17 +114,30 @@ class Meta: "rules", "metadata", ] + read_only_fields = ["project"] + + @cached_property + def project(self) -> Project: + """Resolve the project from the view's URL kwargs""" + if self.instance: + return self.instance.project + try: + pk = int(self.context["view"].kwargs["project_pk"]) + except KeyError as error: + raise RuntimeError( + "The serializer context is missing a view with project_pk in its URL." + ) from error + return Project.objects.get(pk=pk) # type: ignore[no-any-return] def validate(self, attrs: dict[str, Any]) -> dict[str, Any]: attrs = super().validate(attrs) - project = self.instance.project if self.instance else attrs["project"] # type: ignore[union-attr] - organisation = project.organisation + organisation = self.project.organisation self._validate_required_metadata( - organisation, attrs.get("metadata", []), project=project + organisation, attrs.get("metadata", []), project=self.project ) self._validate_segment_rules_conditions_limit(attrs["rules"]) - self._validate_project_segment_limit(project) + self._validate_project_segment_limit(self.project) return attrs def create(self, validated_data: dict[str, Any]): # type: ignore[no-untyped-def] diff --git a/api/segments/views.py b/api/segments/views.py index 91328d8b9aa8..713c0eda32c9 100644 --- a/api/segments/views.py +++ b/api/segments/views.py @@ -135,6 +135,9 @@ def get_queryset(self): # type: ignore[no-untyped-def] return queryset + def perform_create(self, serializer: SegmentSerializer) -> None: # type: ignore[override] + serializer.save(project_id=self.kwargs["project_pk"]) # type: ignore[no-untyped-call] + @extend_schema(parameters=[AssociatedFeaturesQuerySerializer]) @action( detail=True, diff --git a/api/tests/unit/segments/test_unit_segments_views.py b/api/tests/unit/segments/test_unit_segments_views.py index 5557498e14df..eae77b2b0d7c 100644 --- a/api/tests/unit/segments/test_unit_segments_views.py +++ b/api/tests/unit/segments/test_unit_segments_views.py @@ -1884,3 +1884,26 @@ def test_create_segment__required_metadata_on_other_project__returns_201( # Then assert response.status_code == status.HTTP_201_CREATED + + +def test_create_segment__body_project_differs_from_url__does_not_create_in_other_project( + admin_client: APIClient, + project: Project, +) -> None: + # Given + other_org = Organisation.objects.create(name="Other Org") + other_project = Project.objects.create(name="Other Project", organisation=other_org) + + # When + admin_client.post( + f"/api/v1/projects/{project.id}/segments/", + data={ + "name": "a_wild_pokemon", + "project": other_project.id, + "rules": [{"type": "ALL", "rules": [], "conditions": []}], + }, + format="json", + ) + + # Then + assert not Segment.objects.filter(project=other_project).exists() From 6e5a7099b1d10b182149f47f5f429a50fad90dfd Mon Sep 17 00:00:00 2001 From: Evandro Myller Date: Wed, 11 Mar 2026 23:48:28 -0300 Subject: [PATCH 2/5] Compromise on perform_create --- api/segments/serializers.py | 28 +++++++------------ api/segments/views.py | 2 +- .../unit/segments/test_unit_segments_views.py | 4 ++- 3 files changed, 14 insertions(+), 20 deletions(-) diff --git a/api/segments/serializers.py b/api/segments/serializers.py index eba37d9ac4a6..ddf115c03b0d 100644 --- a/api/segments/serializers.py +++ b/api/segments/serializers.py @@ -1,4 +1,3 @@ -from functools import cached_property from typing import Any import structlog @@ -114,30 +113,23 @@ class Meta: "rules", "metadata", ] - read_only_fields = ["project"] - - @cached_property - def project(self) -> Project: - """Resolve the project from the view's URL kwargs""" - if self.instance: - return self.instance.project - try: - pk = int(self.context["view"].kwargs["project_pk"]) - except KeyError as error: - raise RuntimeError( - "The serializer context is missing a view with project_pk in its URL." - ) from error - return Project.objects.get(pk=pk) # type: ignore[no-any-return] def validate(self, attrs: dict[str, Any]) -> dict[str, Any]: attrs = super().validate(attrs) - organisation = self.project.organisation + project = ( + self.instance.project + if self.instance + else Project.objects.get( + pk=int(self.context["view"].kwargs["project_pk"]), + ) + ) + organisation = project.organisation self._validate_required_metadata( - organisation, attrs.get("metadata", []), project=self.project + organisation, attrs.get("metadata", []), project=project ) self._validate_segment_rules_conditions_limit(attrs["rules"]) - self._validate_project_segment_limit(self.project) + self._validate_project_segment_limit(project) return attrs def create(self, validated_data: dict[str, Any]): # type: ignore[no-untyped-def] diff --git a/api/segments/views.py b/api/segments/views.py index 713c0eda32c9..a163842677a2 100644 --- a/api/segments/views.py +++ b/api/segments/views.py @@ -136,7 +136,7 @@ def get_queryset(self): # type: ignore[no-untyped-def] return queryset def perform_create(self, serializer: SegmentSerializer) -> None: # type: ignore[override] - serializer.save(project_id=self.kwargs["project_pk"]) # type: ignore[no-untyped-call] + serializer.save(project_id=int(self.kwargs["project_pk"])) # type: ignore[no-untyped-call] @extend_schema(parameters=[AssociatedFeaturesQuerySerializer]) @action( diff --git a/api/tests/unit/segments/test_unit_segments_views.py b/api/tests/unit/segments/test_unit_segments_views.py index eae77b2b0d7c..d8d66628eee3 100644 --- a/api/tests/unit/segments/test_unit_segments_views.py +++ b/api/tests/unit/segments/test_unit_segments_views.py @@ -1895,7 +1895,7 @@ def test_create_segment__body_project_differs_from_url__does_not_create_in_other other_project = Project.objects.create(name="Other Project", organisation=other_org) # When - admin_client.post( + response = admin_client.post( f"/api/v1/projects/{project.id}/segments/", data={ "name": "a_wild_pokemon", @@ -1906,4 +1906,6 @@ def test_create_segment__body_project_differs_from_url__does_not_create_in_other ) # Then + assert response.status_code == status.HTTP_201_CREATED + assert response.json()["project"] == project.id assert not Segment.objects.filter(project=other_project).exists() From 94a1a22a2399b82cbf177840d2a8ed618addf25d Mon Sep 17 00:00:00 2001 From: Evandro Myller Date: Thu, 12 Mar 2026 13:52:06 -0300 Subject: [PATCH 3/5] Better fix --- api/segments/serializers.py | 11 +++----- api/segments/views.py | 28 +++++++++++++------ .../unit/segments/test_unit_segments_views.py | 2 +- 3 files changed, 24 insertions(+), 17 deletions(-) diff --git a/api/segments/serializers.py b/api/segments/serializers.py index ddf115c03b0d..38481a41c7f1 100644 --- a/api/segments/serializers.py +++ b/api/segments/serializers.py @@ -116,15 +116,12 @@ class Meta: def validate(self, attrs: dict[str, Any]) -> dict[str, Any]: attrs = super().validate(attrs) - project = ( - self.instance.project - if self.instance - else Project.objects.get( - pk=int(self.context["view"].kwargs["project_pk"]), - ) - ) + project: Project = self.context["project"] organisation = project.organisation + # TODO: Make "project" read-only — https://github.com/Flagsmith/flagsmith-workflows/issues/102 + attrs["project"] = project + self._validate_required_metadata( organisation, attrs.get("metadata", []), project=project ) diff --git a/api/segments/views.py b/api/segments/views.py index a163842677a2..acc970a3e4f6 100644 --- a/api/segments/views.py +++ b/api/segments/views.py @@ -1,5 +1,6 @@ import logging -from typing import Any +from functools import cache +from typing import TYPE_CHECKING, Any from common.projects.permissions import VIEW_PROJECT from django.utils.decorators import method_decorator @@ -21,6 +22,7 @@ SegmentAssociatedFeatureStateSerializer, ) from features.versioning.models import EnvironmentFeatureVersion +from projects.models import Project from .models import Segment from .permissions import SegmentPermissions @@ -31,6 +33,9 @@ ) from .services import delete_segment +if TYPE_CHECKING: + from users.models import FFAdminUser + logger = logging.getLogger() @@ -88,15 +93,23 @@ class SegmentViewSet(viewsets.ModelViewSet): # type: ignore[type-arg] permission_classes = [SegmentPermissions] pagination_class = CustomPagination + @cache + def get_project(self) -> Project: + user: "FFAdminUser" = self.request.user # type: ignore[assignment] + projects = user.get_permitted_projects(permission_key=VIEW_PROJECT) + return get_object_or_404(projects, pk=self.kwargs["project_pk"]) + + def get_serializer_context(self) -> dict[str, Any]: + context = super().get_serializer_context() + if not getattr(self, "swagger_fake_view", False): + context["project"] = self.get_project() + return context + def get_queryset(self): # type: ignore[no-untyped-def] if getattr(self, "swagger_fake_view", False): return Segment.objects.none() - permitted_projects = self.request.user.get_permitted_projects( # type: ignore[union-attr] - permission_key=VIEW_PROJECT - ) - project = get_object_or_404(permitted_projects, pk=self.kwargs["project_pk"]) - + project = self.get_project() queryset = Segment.live_objects.filter(project=project, is_system_segment=False) if self.action == "list": @@ -135,9 +148,6 @@ def get_queryset(self): # type: ignore[no-untyped-def] return queryset - def perform_create(self, serializer: SegmentSerializer) -> None: # type: ignore[override] - serializer.save(project_id=int(self.kwargs["project_pk"])) # type: ignore[no-untyped-call] - @extend_schema(parameters=[AssociatedFeaturesQuerySerializer]) @action( detail=True, diff --git a/api/tests/unit/segments/test_unit_segments_views.py b/api/tests/unit/segments/test_unit_segments_views.py index d8d66628eee3..a9a13ca53582 100644 --- a/api/tests/unit/segments/test_unit_segments_views.py +++ b/api/tests/unit/segments/test_unit_segments_views.py @@ -205,7 +205,7 @@ def test_segments_limit_ignores_old_segment_versions( with_project_permissions: WithProjectPermissionsCallable, ) -> None: # Given - with_project_permissions([MANAGE_SEGMENTS]) # type: ignore[call-arg] + with_project_permissions([MANAGE_SEGMENTS, VIEW_PROJECT]) # type: ignore[call-arg] # let's reduce the max segments allowed to 2 project.max_segments_allowed = 2 From eef717eaa2d524471e885926bfee6592fa280973 Mon Sep 17 00:00:00 2001 From: Evandro Myller Date: Thu, 12 Mar 2026 13:55:54 -0300 Subject: [PATCH 4/5] Needless now --- api/segments/serializers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/api/segments/serializers.py b/api/segments/serializers.py index 38481a41c7f1..38e53e62ebc7 100644 --- a/api/segments/serializers.py +++ b/api/segments/serializers.py @@ -79,7 +79,6 @@ class Meta: class SegmentSerializer(MetadataSerializerMixin, WritableNestedModelSerializer): - instance: Segment | None rules = SegmentRuleSerializer(many=True, required=True, allow_empty=False) metadata = MetadataSerializer(required=False, many=True) From 3a96a9f5d68ceaa7582f0db6ec350a5063346768 Mon Sep 17 00:00:00 2001 From: Evandro Myller Date: Thu, 12 Mar 2026 15:11:48 -0300 Subject: [PATCH 5/5] Compatible fix --- api/segments/serializers.py | 11 +++++------ api/segments/views.py | 8 -------- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/api/segments/serializers.py b/api/segments/serializers.py index 38e53e62ebc7..bd9397a402a1 100644 --- a/api/segments/serializers.py +++ b/api/segments/serializers.py @@ -115,15 +115,14 @@ class Meta: def validate(self, attrs: dict[str, Any]) -> dict[str, Any]: attrs = super().validate(attrs) - project: Project = self.context["project"] - organisation = project.organisation + metadata = attrs.get("metadata", []) # TODO: Make "project" read-only — https://github.com/Flagsmith/flagsmith-workflows/issues/102 - attrs["project"] = project + project_pk = self.context["view"].kwargs["project_pk"] + project = attrs["project"] = Project.objects.get(pk=project_pk) + organisation = project.organisation - self._validate_required_metadata( - organisation, attrs.get("metadata", []), project=project - ) + self._validate_required_metadata(organisation, metadata, project) self._validate_segment_rules_conditions_limit(attrs["rules"]) self._validate_project_segment_limit(project) return attrs diff --git a/api/segments/views.py b/api/segments/views.py index acc970a3e4f6..a6016abb4c96 100644 --- a/api/segments/views.py +++ b/api/segments/views.py @@ -1,5 +1,4 @@ import logging -from functools import cache from typing import TYPE_CHECKING, Any from common.projects.permissions import VIEW_PROJECT @@ -93,18 +92,11 @@ class SegmentViewSet(viewsets.ModelViewSet): # type: ignore[type-arg] permission_classes = [SegmentPermissions] pagination_class = CustomPagination - @cache def get_project(self) -> Project: user: "FFAdminUser" = self.request.user # type: ignore[assignment] projects = user.get_permitted_projects(permission_key=VIEW_PROJECT) return get_object_or_404(projects, pk=self.kwargs["project_pk"]) - def get_serializer_context(self) -> dict[str, Any]: - context = super().get_serializer_context() - if not getattr(self, "swagger_fake_view", False): - context["project"] = self.get_project() - return context - def get_queryset(self): # type: ignore[no-untyped-def] if getattr(self, "swagger_fake_view", False): return Segment.objects.none()