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 diff --git a/src/sentry/integrations/api/endpoints/data_forwarding_details.py b/src/sentry/integrations/api/endpoints/data_forwarding_details.py index 7152ccac136035..1afec9fc9aadf9 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 @@ -78,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, diff --git a/src/sentry/integrations/api/endpoints/data_forwarding_index.py b/src/sentry/integrations/api/endpoints/data_forwarding_index.py index 985d4c3fdcf4c4..44538b9d5e54eb 100644 --- a/src/sentry/integrations/api/endpoints/data_forwarding_index.py +++ b/src/sentry/integrations/api/endpoints/data_forwarding_index.py @@ -2,9 +2,11 @@ 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 +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 @@ -40,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], @@ -73,6 +81,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", 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..3c07f9964f195c 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,42 @@ 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, + "organizations:data-forwarding": True, + } + ): + return super().get_response(*args, **kwargs) + @region_silo_test class DataForwardingIndexGetTest(DataForwardingIndexEndpointTest): + + 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( provider=DataForwarderProviderSlug.SEGMENT, @@ -123,6 +158,26 @@ def test_get_with_disabled_data_forwarder(self) -> None: class DataForwardingIndexPostTest(DataForwardingIndexEndpointTest): method = "POST" + 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 = { "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..7dbad601bab247 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,57 @@ 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, + "organizations:data-forwarding": True, + } + ): + return super().get_response(*args, **kwargs) + @region_silo_test class DataForwardingDetailsPutTest(DataForwardingDetailsEndpointTest): method = "PUT" + 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, + ) + 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, + ) + 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( provider=DataForwarderProviderSlug.SEGMENT, @@ -37,9 +85,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 +507,40 @@ def test_update_single_project_creates_new_config(self) -> None: class DataForwardingDetailsDeleteTest(DataForwardingDetailsEndpointTest): method = "DELETE" + 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, + ) + 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, + ) + 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( provider=DataForwarderProviderSlug.SEGMENT,