From 49f7e7ba839ccf8591a42b8eb9aeb2ff5007a3bf Mon Sep 17 00:00:00 2001 From: Leander Rodrigues Date: Tue, 4 Nov 2025 12:18:42 -0500 Subject: [PATCH 1/3] =?UTF-8?q?=E2=9C=A8=20add=20feature=20for=20data-forw?= =?UTF-8?q?arding=20revamp=20access?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/sentry/features/temporary.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/sentry/features/temporary.py b/src/sentry/features/temporary.py index 4ab140ea21c31c..163ac2fe4462b2 100644 --- a/src/sentry/features/temporary.py +++ b/src/sentry/features/temporary.py @@ -122,6 +122,10 @@ def register_temporary_features(manager: FeatureManager) -> None: manager.add("organizations:data-secrecy", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True) # Data Secrecy v2 (with Break the Glass feature) manager.add("organizations:data-secrecy-v2", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True) + # Enables access to the global data forwarding features + # Note: organizations:data-forwarding is a permanent, plan-based flag that enables access to the actual feature. + # This flag will only be used to access the new UI/UX and endpoints before release. + manager.add("organizations:data-forwarding-revamp-access", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True) # Enables synthesis of device.class in ingest manager.add("organizations:device-class-synthesis", OrganizationFeature, FeatureHandlerStrategy.INTERNAL, api_expose=False) # Enable device.class as a selectable column From f2eb8ca640cf5e5711bee6ba5ab04b8eedead5cd Mon Sep 17 00:00:00 2001 From: Leander Rodrigues Date: Thu, 6 Nov 2025 11:17:54 -0500 Subject: [PATCH 2/3] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20limit=20endpoint=20acc?= =?UTF-8?q?ess=20with=20flag?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../api/endpoints/data_forwarding_details.py | 7 ++++ .../api/endpoints/data_forwarding_index.py | 7 ++++ .../api/endpoints/test_data_forwarding.py | 18 +++++++++ .../endpoints/test_data_forwarding_details.py | 38 +++++++++++++++++-- 4 files changed, 67 insertions(+), 3 deletions(-) diff --git a/src/sentry/integrations/api/endpoints/data_forwarding_details.py b/src/sentry/integrations/api/endpoints/data_forwarding_details.py index 7152ccac136035..3398b49e421bf0 100644 --- a/src/sentry/integrations/api/endpoints/data_forwarding_details.py +++ b/src/sentry/integrations/api/endpoints/data_forwarding_details.py @@ -10,6 +10,7 @@ from rest_framework.response import Response from rest_framework.views import APIView +from sentry import features from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import region_silo_endpoint @@ -286,6 +287,9 @@ def _update_single_project_configuration( def put( self, request: Request, organization: Organization, data_forwarder: DataForwarder ) -> Response: + if not features.has("organizations:data-forwarding-revamp-access", organization): + return self.respond(status=status.HTTP_403_FORBIDDEN) + # org:write users can update the main data forwarder configuration if request.access.has_scope("org:write"): return self._update_data_forwarder_config(request, organization, data_forwarder) @@ -323,5 +327,8 @@ def put( def delete( self, request: Request, organization: Organization, data_forwarder: DataForwarder ) -> Response: + if not features.has("organizations:data-forwarding-revamp-access", organization): + return self.respond(status=status.HTTP_403_FORBIDDEN) + data_forwarder.delete() return self.respond(status=status.HTTP_204_NO_CONTENT) diff --git a/src/sentry/integrations/api/endpoints/data_forwarding_index.py b/src/sentry/integrations/api/endpoints/data_forwarding_index.py index 985d4c3fdcf4c4..c78352dc822f7c 100644 --- a/src/sentry/integrations/api/endpoints/data_forwarding_index.py +++ b/src/sentry/integrations/api/endpoints/data_forwarding_index.py @@ -5,6 +5,7 @@ from rest_framework.request import Request from rest_framework.response import Response +from sentry import features from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import region_silo_endpoint @@ -50,6 +51,9 @@ class DataForwardingIndexEndpoint(OrganizationEndpoint): @set_referrer_policy("strict-origin-when-cross-origin") @method_decorator(never_cache) def get(self, request: Request, organization) -> Response: + if not features.has("organizations:data-forwarding-revamp-access", organization): + return self.respond(status=status.HTTP_403_FORBIDDEN) + queryset = DataForwarder.objects.filter(organization_id=organization.id) return self.paginate( @@ -73,6 +77,9 @@ def get(self, request: Request, organization) -> Response: @set_referrer_policy("strict-origin-when-cross-origin") @method_decorator(never_cache) def post(self, request: Request, organization) -> Response: + if not features.has("organizations:data-forwarding-revamp-access", organization): + return self.respond(status=status.HTTP_403_FORBIDDEN) + data = request.data data["organization_id"] = organization.id diff --git a/tests/sentry/integrations/api/endpoints/test_data_forwarding.py b/tests/sentry/integrations/api/endpoints/test_data_forwarding.py index 8ca22cb6549189..8ced3d331c2f9c 100644 --- a/tests/sentry/integrations/api/endpoints/test_data_forwarding.py +++ b/tests/sentry/integrations/api/endpoints/test_data_forwarding.py @@ -1,3 +1,5 @@ +from django.urls import reverse + from sentry.integrations.models.data_forwarder import DataForwarder from sentry.integrations.types import DataForwarderProviderSlug from sentry.testutils.cases import APITestCase @@ -12,9 +14,21 @@ def setUp(self) -> None: super().setUp() self.login_as(user=self.user) + def get_response(self, *args, **kwargs): + """ + Override get_response to always add the required feature flag. + """ + with self.feature({"organizations:data-forwarding-revamp-access": True}): + return super().get_response(*args, **kwargs) + @region_silo_test class DataForwardingIndexGetTest(DataForwardingIndexEndpointTest): + + def test_without_feature_flag_access(self) -> None: + response = self.client.get(reverse(self.endpoint, args=(self.organization.slug,))) + assert response.status_code == 403 + def test_get_single_data_forwarder(self) -> None: data_forwarder = self.create_data_forwarder( provider=DataForwarderProviderSlug.SEGMENT, @@ -123,6 +137,10 @@ def test_get_with_disabled_data_forwarder(self) -> None: class DataForwardingIndexPostTest(DataForwardingIndexEndpointTest): method = "POST" + def test_without_feature_flag_access(self) -> None: + response = self.client.post(reverse(self.endpoint, args=(self.organization.slug,))) + assert response.status_code == 403 + def test_create_segment_data_forwarder(self) -> None: payload = { "provider": DataForwarderProviderSlug.SEGMENT, diff --git a/tests/sentry/integrations/api/endpoints/test_data_forwarding_details.py b/tests/sentry/integrations/api/endpoints/test_data_forwarding_details.py index 9143fbbdec76df..9f51d92d8f37f8 100644 --- a/tests/sentry/integrations/api/endpoints/test_data_forwarding_details.py +++ b/tests/sentry/integrations/api/endpoints/test_data_forwarding_details.py @@ -1,3 +1,5 @@ +from django.urls import reverse + from sentry.integrations.models.data_forwarder import DataForwarder from sentry.integrations.models.data_forwarder_project import DataForwarderProject from sentry.integrations.types import DataForwarderProviderSlug @@ -13,11 +15,29 @@ def setUp(self) -> None: super().setUp() self.login_as(user=self.user) + def get_response(self, *args, **kwargs): + """ + Override get_response to always add the required feature flag. + """ + with self.feature({"organizations:data-forwarding-revamp-access": True}): + return super().get_response(*args, **kwargs) + @region_silo_test class DataForwardingDetailsPutTest(DataForwardingDetailsEndpointTest): method = "PUT" + def test_without_feature_flag_access(self) -> None: + data_forwarder = self.create_data_forwarder( + provider=DataForwarderProviderSlug.SEGMENT, + config={"write_key": "old_key"}, + is_enabled=True, + ) + response = self.client.put( + reverse(self.endpoint, args=(self.organization.slug, data_forwarder.id)) + ) + assert response.status_code == 403 + def test_update_data_forwarder(self) -> None: data_forwarder = self.create_data_forwarder( provider=DataForwarderProviderSlug.SEGMENT, @@ -37,9 +57,10 @@ def test_update_data_forwarder(self) -> None: "project_ids": [self.project.id], } - response = self.get_success_response( - self.organization.slug, data_forwarder.id, status_code=200, **payload - ) + with self.feature({"organizations:data-forwarding-revamp-access": True}): + response = self.get_success_response( + self.organization.slug, data_forwarder.id, status_code=200, **payload + ) assert response.data["config"] == {"write_key": "new_key"} assert not response.data["isEnabled"] @@ -458,6 +479,17 @@ def test_update_single_project_creates_new_config(self) -> None: class DataForwardingDetailsDeleteTest(DataForwardingDetailsEndpointTest): method = "DELETE" + def test_without_feature_flag_access(self) -> None: + data_forwarder = self.create_data_forwarder( + provider=DataForwarderProviderSlug.SEGMENT, + config={"write_key": "old_key"}, + is_enabled=True, + ) + response = self.client.delete( + reverse(self.endpoint, args=(self.organization.slug, data_forwarder.id)) + ) + assert response.status_code == 403 + def test_delete_data_forwarder(self) -> None: data_forwarder = self.create_data_forwarder( provider=DataForwarderProviderSlug.SEGMENT, From bcda6f2c81fd341d4d9fc199ab9a100c143f3cbd Mon Sep 17 00:00:00 2001 From: Leander Rodrigues Date: Thu, 6 Nov 2025 11:46:46 -0500 Subject: [PATCH 3/3] =?UTF-8?q?=E2=9C=A8=20check=20second=20flag,=20allow?= =?UTF-8?q?=20lower=20perms=20for=20certain=20methods?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../api/endpoints/data_forwarding_details.py | 14 ++-- .../api/endpoints/data_forwarding_index.py | 12 ++-- .../api/endpoints/test_data_forwarding.py | 51 ++++++++++++-- .../endpoints/test_data_forwarding_details.py | 69 ++++++++++++++++--- 4 files changed, 120 insertions(+), 26 deletions(-) diff --git a/src/sentry/integrations/api/endpoints/data_forwarding_details.py b/src/sentry/integrations/api/endpoints/data_forwarding_details.py index 3398b49e421bf0..1afec9fc9aadf9 100644 --- a/src/sentry/integrations/api/endpoints/data_forwarding_details.py +++ b/src/sentry/integrations/api/endpoints/data_forwarding_details.py @@ -79,6 +79,14 @@ def convert_args( ): args, kwargs = super().convert_args(request, organization_id_or_slug, *args, **kwargs) + if not features.has("organizations:data-forwarding-revamp-access", kwargs["organization"]): + raise PermissionDenied + + if request.method == "PUT" and not features.has( + "organizations:data-forwarding", kwargs["organization"] + ): + raise PermissionDenied + try: data_forwarder = DataForwarder.objects.get( id=data_forwarder_id, @@ -287,9 +295,6 @@ def _update_single_project_configuration( def put( self, request: Request, organization: Organization, data_forwarder: DataForwarder ) -> Response: - if not features.has("organizations:data-forwarding-revamp-access", organization): - return self.respond(status=status.HTTP_403_FORBIDDEN) - # org:write users can update the main data forwarder configuration if request.access.has_scope("org:write"): return self._update_data_forwarder_config(request, organization, data_forwarder) @@ -327,8 +332,5 @@ def put( def delete( self, request: Request, organization: Organization, data_forwarder: DataForwarder ) -> Response: - if not features.has("organizations:data-forwarding-revamp-access", organization): - return self.respond(status=status.HTTP_403_FORBIDDEN) - data_forwarder.delete() return self.respond(status=status.HTTP_204_NO_CONTENT) diff --git a/src/sentry/integrations/api/endpoints/data_forwarding_index.py b/src/sentry/integrations/api/endpoints/data_forwarding_index.py index c78352dc822f7c..44538b9d5e54eb 100644 --- a/src/sentry/integrations/api/endpoints/data_forwarding_index.py +++ b/src/sentry/integrations/api/endpoints/data_forwarding_index.py @@ -2,6 +2,7 @@ from django.views.decorators.cache import never_cache from drf_spectacular.utils import extend_schema from rest_framework import status +from rest_framework.exceptions import PermissionDenied from rest_framework.request import Request from rest_framework.response import Response @@ -41,6 +42,12 @@ class DataForwardingIndexEndpoint(OrganizationEndpoint): } permission_classes = (OrganizationDataForwardingDetailsPermission,) + def convert_args(self, request: Request, *args, **kwargs): + args, kwargs = super().convert_args(request, *args, **kwargs) + if not features.has("organizations:data-forwarding-revamp-access", kwargs["organization"]): + raise PermissionDenied + return args, kwargs + @extend_schema( operation_id="Retrieve Data Forwarding Configurations for an Organization", parameters=[GlobalParams.ORG_ID_OR_SLUG], @@ -51,9 +58,6 @@ class DataForwardingIndexEndpoint(OrganizationEndpoint): @set_referrer_policy("strict-origin-when-cross-origin") @method_decorator(never_cache) def get(self, request: Request, organization) -> Response: - if not features.has("organizations:data-forwarding-revamp-access", organization): - return self.respond(status=status.HTTP_403_FORBIDDEN) - queryset = DataForwarder.objects.filter(organization_id=organization.id) return self.paginate( @@ -77,7 +81,7 @@ def get(self, request: Request, organization) -> Response: @set_referrer_policy("strict-origin-when-cross-origin") @method_decorator(never_cache) def post(self, request: Request, organization) -> Response: - if not features.has("organizations:data-forwarding-revamp-access", organization): + if not features.has("organizations:data-forwarding", organization): return self.respond(status=status.HTTP_403_FORBIDDEN) data = request.data diff --git a/tests/sentry/integrations/api/endpoints/test_data_forwarding.py b/tests/sentry/integrations/api/endpoints/test_data_forwarding.py index 8ced3d331c2f9c..3c07f9964f195c 100644 --- a/tests/sentry/integrations/api/endpoints/test_data_forwarding.py +++ b/tests/sentry/integrations/api/endpoints/test_data_forwarding.py @@ -18,16 +18,37 @@ def get_response(self, *args, **kwargs): """ Override get_response to always add the required feature flag. """ - with self.feature({"organizations:data-forwarding-revamp-access": True}): + with self.feature( + { + "organizations:data-forwarding-revamp-access": True, + "organizations:data-forwarding": True, + } + ): return super().get_response(*args, **kwargs) @region_silo_test class DataForwardingIndexGetTest(DataForwardingIndexEndpointTest): - def test_without_feature_flag_access(self) -> None: - response = self.client.get(reverse(self.endpoint, args=(self.organization.slug,))) - assert response.status_code == 403 + def test_without_revamp_feature_flag_access(self) -> None: + with self.feature( + { + "organizations:data-forwarding-revamp-access": False, + "organizations:data-forwarding": True, + } + ): + response = self.client.get(reverse(self.endpoint, args=(self.organization.slug,))) + assert response.status_code == 403 + + def test_without_data_forwarding_feature_flag_access(self) -> None: + with self.feature( + { + "organizations:data-forwarding-revamp-access": True, + "organizations:data-forwarding": False, + } + ): + response = self.client.get(reverse(self.endpoint, args=(self.organization.slug,))) + assert response.status_code == 200 def test_get_single_data_forwarder(self) -> None: data_forwarder = self.create_data_forwarder( @@ -137,9 +158,25 @@ def test_get_with_disabled_data_forwarder(self) -> None: class DataForwardingIndexPostTest(DataForwardingIndexEndpointTest): method = "POST" - def test_without_feature_flag_access(self) -> None: - response = self.client.post(reverse(self.endpoint, args=(self.organization.slug,))) - assert response.status_code == 403 + def test_without_revamp_feature_flag_access(self) -> None: + with self.feature( + { + "organizations:data-forwarding-revamp-access": False, + "organizations:data-forwarding": True, + } + ): + response = self.client.post(reverse(self.endpoint, args=(self.organization.slug,))) + assert response.status_code == 403 + + def test_without_data_forwarding_feature_flag_access(self) -> None: + with self.feature( + { + "organizations:data-forwarding-revamp-access": True, + "organizations:data-forwarding": False, + } + ): + response = self.client.post(reverse(self.endpoint, args=(self.organization.slug,))) + assert response.status_code == 403 def test_create_segment_data_forwarder(self) -> None: payload = { diff --git a/tests/sentry/integrations/api/endpoints/test_data_forwarding_details.py b/tests/sentry/integrations/api/endpoints/test_data_forwarding_details.py index 9f51d92d8f37f8..7dbad601bab247 100644 --- a/tests/sentry/integrations/api/endpoints/test_data_forwarding_details.py +++ b/tests/sentry/integrations/api/endpoints/test_data_forwarding_details.py @@ -19,7 +19,12 @@ def get_response(self, *args, **kwargs): """ Override get_response to always add the required feature flag. """ - with self.feature({"organizations:data-forwarding-revamp-access": True}): + with self.feature( + { + "organizations:data-forwarding-revamp-access": True, + "organizations:data-forwarding": True, + } + ): return super().get_response(*args, **kwargs) @@ -27,16 +32,39 @@ def get_response(self, *args, **kwargs): class DataForwardingDetailsPutTest(DataForwardingDetailsEndpointTest): method = "PUT" - def test_without_feature_flag_access(self) -> None: + def test_without_revamp_feature_flag_access(self) -> None: data_forwarder = self.create_data_forwarder( provider=DataForwarderProviderSlug.SEGMENT, config={"write_key": "old_key"}, is_enabled=True, ) - response = self.client.put( - reverse(self.endpoint, args=(self.organization.slug, data_forwarder.id)) + with self.feature( + { + "organizations:data-forwarding-revamp-access": False, + "organizations:data-forwarding": True, + } + ): + response = self.client.put( + reverse(self.endpoint, args=(self.organization.slug, data_forwarder.id)) + ) + assert response.status_code == 403 + + def test_without_data_forwarding_feature_flag_access(self) -> None: + data_forwarder = self.create_data_forwarder( + provider=DataForwarderProviderSlug.SEGMENT, + config={"write_key": "old_key"}, + is_enabled=True, ) - assert response.status_code == 403 + with self.feature( + { + "organizations:data-forwarding-revamp-access": True, + "organizations:data-forwarding": False, + } + ): + response = self.client.put( + reverse(self.endpoint, args=(self.organization.slug, data_forwarder.id)) + ) + assert response.status_code == 403 def test_update_data_forwarder(self) -> None: data_forwarder = self.create_data_forwarder( @@ -479,16 +507,39 @@ def test_update_single_project_creates_new_config(self) -> None: class DataForwardingDetailsDeleteTest(DataForwardingDetailsEndpointTest): method = "DELETE" - def test_without_feature_flag_access(self) -> None: + def test_without_revamp_feature_flag_access(self) -> None: data_forwarder = self.create_data_forwarder( provider=DataForwarderProviderSlug.SEGMENT, config={"write_key": "old_key"}, is_enabled=True, ) - response = self.client.delete( - reverse(self.endpoint, args=(self.organization.slug, data_forwarder.id)) + with self.feature( + { + "organizations:data-forwarding-revamp-access": False, + "organizations:data-forwarding": True, + } + ): + response = self.client.delete( + reverse(self.endpoint, args=(self.organization.slug, data_forwarder.id)) + ) + assert response.status_code == 403 + + def test_without_data_forwarding_feature_flag_access(self) -> None: + data_forwarder = self.create_data_forwarder( + provider=DataForwarderProviderSlug.SEGMENT, + config={"write_key": "old_key"}, + is_enabled=True, ) - assert response.status_code == 403 + with self.feature( + { + "organizations:data-forwarding-revamp-access": True, + "organizations:data-forwarding": False, + } + ): + response = self.client.delete( + reverse(self.endpoint, args=(self.organization.slug, data_forwarder.id)) + ) + assert response.status_code == 204 def test_delete_data_forwarder(self) -> None: data_forwarder = self.create_data_forwarder(