From 118f34fe590771b41d1605859a298469188a2da3 Mon Sep 17 00:00:00 2001 From: Shashank Jarmale Date: Thu, 11 Jun 2026 14:49:24 -0700 Subject: [PATCH 01/12] Implement monitoring provider connection API endpoints --- .../organization_monitoring_providers.py | 151 +++++++++++ src/sentry/api/urls.py | 14 + .../test_organization_monitoring_providers.py | 254 ++++++++++++++++++ 3 files changed, 419 insertions(+) create mode 100644 src/sentry/api/endpoints/organization_monitoring_providers.py create mode 100644 tests/sentry/api/endpoints/test_organization_monitoring_providers.py diff --git a/src/sentry/api/endpoints/organization_monitoring_providers.py b/src/sentry/api/endpoints/organization_monitoring_providers.py new file mode 100644 index 00000000000000..567250c24dadeb --- /dev/null +++ b/src/sentry/api/endpoints/organization_monitoring_providers.py @@ -0,0 +1,151 @@ +from __future__ import annotations + +import logging + +from django.http import HttpResponseRedirect +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 control_silo_endpoint +from sentry.api.bases.organization import ControlSiloOrganizationEndpoint +from sentry.identity.pipeline import IdentityPipeline +from sentry.organizations.services.organization.model import RpcOrganization +from sentry.users.models.identity import Identity, IdentityProvider + +logger = logging.getLogger(__name__) + +MONITORING_PROVIDERS: dict[str, dict[str, str]] = { + "datadog": {"name": "Datadog"}, + "gcp": {"name": "Google Cloud Platform"}, +} + +MONITORING_PROVIDER_FEATURE = "organizations:seer-infra-telemetry" + + +def _get_pipeline_config(provider_key: str, request_data: dict) -> dict: + """Build provider-specific pipeline config from request body.""" + config: dict[str, str] = {} + if provider_key == "datadog": + site = request_data.get("site") + if not site: + raise ValueError("Datadog requires a 'site' parameter (e.g. 'datadoghq.com').") + config["site"] = site + return config + + +def _get_identity_provider(provider_key: str, config: dict) -> IdentityProvider: + """ + Get or create the IdentityProvider for a monitoring provider. + + Datadog uses per-site providers (external_id=site). GCP uses a single global provider. + """ + if provider_key == "datadog": + external_id = config.get("site") + else: + external_id = None + + idp, _ = IdentityProvider.objects.get_or_create(type=provider_key, external_id=external_id) + return idp + + +@control_silo_endpoint +class OrganizationMonitoringProviderIndexEndpoint(ControlSiloOrganizationEndpoint): + owner = ApiOwner.CODING_WORKFLOWS + publish_status = { + "GET": ApiPublishStatus.PRIVATE, + } + + def get(self, request: Request, organization: RpcOrganization, **kwargs: object) -> Response: + if not features.has(MONITORING_PROVIDER_FEATURE, organization, actor=request.user): + return Response(status=404) + + connected_identities = { + identity.idp.type: identity + for identity in Identity.objects.filter( + idp__type__in=MONITORING_PROVIDERS.keys(), + user_id=request.user.id, + ).select_related("idp") + } + + providers = [] + for key, meta in MONITORING_PROVIDERS.items(): + identity = connected_identities.get(key) + entry: dict[str, str | bool] = { + "provider": key, + "name": meta["name"], + "connected": identity is not None, + } + if identity is not None: + email = identity.data.get("email") if identity.data else None + if email: + entry["email"] = email + providers.append(entry) + + return Response({"providers": providers}) + + +@control_silo_endpoint +class OrganizationMonitoringProviderDetailsEndpoint(ControlSiloOrganizationEndpoint): + owner = ApiOwner.CODING_WORKFLOWS + publish_status = { + "POST": ApiPublishStatus.PRIVATE, + "DELETE": ApiPublishStatus.PRIVATE, + } + + def post( + self, request: Request, organization: RpcOrganization, provider_key: str, **kwargs: object + ) -> Response: + if not features.has(MONITORING_PROVIDER_FEATURE, organization, actor=request.user): + return Response(status=404) + + if provider_key not in MONITORING_PROVIDERS: + return Response({"detail": "Unknown monitoring provider."}, status=400) + + try: + config = _get_pipeline_config(provider_key, request.data) + except ValueError as e: + return Response({"detail": str(e)}, status=400) + + idp = _get_identity_provider(provider_key, config) + + pipeline = IdentityPipeline( + request=request._request, + provider_key=provider_key, + organization=organization, + provider_model=idp, + config=config, + ) + pipeline.initialize() + + response = pipeline.current_step() + + if isinstance(response, HttpResponseRedirect): + return Response({"redirectUrl": response.url}) + + logger.error( + "monitoring_provider.connect.unexpected_response", + extra={"provider": provider_key, "response_type": type(response).__name__}, + ) + return Response({"detail": "Failed to start OAuth flow."}, status=500) + + def delete( + self, request: Request, organization: RpcOrganization, provider_key: str, **kwargs: object + ) -> Response: + if not features.has(MONITORING_PROVIDER_FEATURE, organization, actor=request.user): + return Response(status=404) + + if provider_key not in MONITORING_PROVIDERS: + return Response({"detail": "Unknown monitoring provider."}, status=400) + + deleted, _ = Identity.objects.filter( + idp__type=provider_key, + user_id=request.user.id, + ).delete() + + if not deleted: + return Response({"detail": "Not connected to this provider."}, status=404) + + return Response(status=204) diff --git a/src/sentry/api/urls.py b/src/sentry/api/urls.py index 56e30a0ba8b1f2..e32f997518c67f 100644 --- a/src/sentry/api/urls.py +++ b/src/sentry/api/urls.py @@ -19,6 +19,10 @@ from sentry.api.endpoints.organization_insights_tree import OrganizationInsightsTreeEndpoint from sentry.api.endpoints.organization_intercom_jwt import OrganizationIntercomJwtEndpoint from sentry.api.endpoints.organization_missing_org_members import OrganizationMissingMembersEndpoint +from sentry.api.endpoints.organization_monitoring_providers import ( + OrganizationMonitoringProviderDetailsEndpoint, + OrganizationMonitoringProviderIndexEndpoint, +) from sentry.api.endpoints.organization_pipeline import OrganizationPipelineEndpoint from sentry.api.endpoints.organization_plugin_deprecation_info import ( OrganizationPluginDeprecationInfoEndpoint, @@ -1901,6 +1905,16 @@ def create_group_urls(name_prefix: str) -> list[URLPattern | URLResolver]: OrganizationIssueTimeSeriesEndpoint.as_view(), name="sentry-api-0-organization-issue-timeseries", ), + re_path( + r"^(?P[^/]+)/monitoring-providers/$", + OrganizationMonitoringProviderIndexEndpoint.as_view(), + name="sentry-api-0-organization-monitoring-providers", + ), + re_path( + r"^(?P[^/]+)/monitoring-providers/(?P[^/]+)/$", + OrganizationMonitoringProviderDetailsEndpoint.as_view(), + name="sentry-api-0-organization-monitoring-provider-details", + ), re_path( r"^(?P[^/]+)/integrations/$", OrganizationIntegrationsEndpoint.as_view(), diff --git a/tests/sentry/api/endpoints/test_organization_monitoring_providers.py b/tests/sentry/api/endpoints/test_organization_monitoring_providers.py new file mode 100644 index 00000000000000..abc05e7aab03e5 --- /dev/null +++ b/tests/sentry/api/endpoints/test_organization_monitoring_providers.py @@ -0,0 +1,254 @@ +from __future__ import annotations + +from unittest.mock import patch + +from django.http import HttpResponseRedirect + +from sentry.testutils.cases import APITestCase +from sentry.testutils.silo import control_silo_test +from sentry.users.models.identity import Identity, IdentityProvider + + +@control_silo_test +class OrganizationMonitoringProviderIndexEndpointTest(APITestCase): + endpoint = "sentry-api-0-organization-monitoring-providers" + method = "get" + + def setUp(self) -> None: + super().setUp() + self.login_as(self.user) + + def test_list_requires_feature_flag(self) -> None: + response = self.get_response(self.organization.slug) + assert response.status_code == 404 + + def test_list_providers(self) -> None: + with self.feature("organizations:seer-infra-telemetry"): + response = self.get_success_response(self.organization.slug) + + providers = {p["provider"]: p for p in response.data["providers"]} + assert "gcp" in providers + assert "datadog" in providers + assert providers["gcp"]["name"] == "Google Cloud Platform" + assert providers["datadog"]["name"] == "Datadog" + assert providers["gcp"]["connected"] is False + assert providers["datadog"]["connected"] is False + + def test_list_shows_connected_provider(self) -> None: + idp = self.create_identity_provider(type="gcp") + self.create_identity( + user=self.user, + identity_provider=idp, + external_id="google-user-123", + data={"email": "user@example.com", "access_token": "token"}, + ) + + with self.feature("organizations:seer-infra-telemetry"): + response = self.get_success_response(self.organization.slug) + + providers = {p["provider"]: p for p in response.data["providers"]} + assert providers["gcp"] == { + "provider": "gcp", + "name": "Google Cloud Platform", + "connected": True, + "email": "user@example.com", + } + assert providers["datadog"] == { + "provider": "datadog", + "name": "Datadog", + "connected": False, + } + + def test_list_connected_without_email(self) -> None: + idp = self.create_identity_provider(type="datadog", external_id="datadoghq.com") + self.create_identity( + user=self.user, + identity_provider=idp, + external_id="dd-user-123", + data={"access_token": "token"}, + ) + + with self.feature("organizations:seer-infra-telemetry"): + response = self.get_success_response(self.organization.slug) + + providers = {p["provider"]: p for p in response.data["providers"]} + assert providers["datadog"]["connected"] is True + assert "email" not in providers["datadog"] + + def test_list_does_not_show_other_users_connections(self) -> None: + other_user = self.create_user() + self.create_member(organization=self.organization, user=other_user) + + idp = self.create_identity_provider(type="gcp") + self.create_identity( + user=other_user, + identity_provider=idp, + external_id="other-google-user-456", + data={"email": "other@example.com", "access_token": "token"}, + ) + + with self.feature("organizations:seer-infra-telemetry"): + response = self.get_success_response(self.organization.slug) + + providers = {p["provider"]: p for p in response.data["providers"]} + assert providers["gcp"]["connected"] is False + assert providers["datadog"]["connected"] is False + + +@control_silo_test +class OrganizationMonitoringProviderDetailsConnectTest(APITestCase): + endpoint = "sentry-api-0-organization-monitoring-provider-details" + method = "post" + + def setUp(self) -> None: + super().setUp() + self.login_as(self.user) + + def test_connect_requires_feature_flag(self) -> None: + response = self.get_response(self.organization.slug, "datadog") + assert response.status_code == 404 + + @patch("sentry.identity.pipeline.IdentityPipeline.current_step") + @patch("sentry.identity.pipeline.IdentityPipeline.initialize") + def test_connect_returns_redirect_url(self, mock_initialize, mock_current_step) -> None: + mock_current_step.return_value = HttpResponseRedirect( + "https://accounts.google.com/o/oauth2/v2/auth?client_id=test" + ) + + with self.feature("organizations:seer-infra-telemetry"): + response = self.get_success_response(self.organization.slug, "gcp") + + assert "redirectUrl" in response.data + assert response.data["redirectUrl"].startswith("https://accounts.google.com/") + mock_initialize.assert_called_once() + + @patch("sentry.identity.pipeline.IdentityPipeline.current_step") + @patch("sentry.identity.pipeline.IdentityPipeline.initialize") + def test_connect_gcp_creates_identity_provider( + self, mock_initialize, mock_current_step + ) -> None: + mock_current_step.return_value = HttpResponseRedirect("https://accounts.google.com/") + + assert not IdentityProvider.objects.filter(type="gcp").exists() + + with self.feature("organizations:seer-infra-telemetry"): + self.get_success_response(self.organization.slug, "gcp") + + assert IdentityProvider.objects.filter(type="gcp").exists() + + @patch("sentry.api.endpoints.organization_monitoring_providers.IdentityPipeline.current_step") + @patch("sentry.api.endpoints.organization_monitoring_providers.IdentityPipeline.initialize") + @patch( + "sentry.api.endpoints.organization_monitoring_providers.IdentityPipeline.__init__", + return_value=None, + ) + def test_connect_datadog_creates_identity_provider( + self, mock_init, mock_initialize, mock_current_step + ) -> None: + mock_current_step.return_value = HttpResponseRedirect( + "https://mcp.datadoghq.com/api/unstable/mcp-server/authorize" + ) + + assert not IdentityProvider.objects.filter(type="datadog").exists() + + with self.feature("organizations:seer-infra-telemetry"): + response = self.get_success_response( + self.organization.slug, "datadog", site="datadoghq.com" + ) + + assert IdentityProvider.objects.filter(type="datadog", external_id="datadoghq.com").exists() + assert response.data["redirectUrl"].startswith("https://mcp.datadoghq.com/") + + def test_connect_datadog_requires_site(self) -> None: + with self.feature("organizations:seer-infra-telemetry"): + response = self.get_response(self.organization.slug, "datadog") + + assert response.status_code == 400 + assert "site" in response.data["detail"] + + def test_connect_unknown_provider(self) -> None: + with self.feature("organizations:seer-infra-telemetry"): + response = self.get_response(self.organization.slug, "unknown") + + assert response.status_code == 400 + + +@control_silo_test +class OrganizationMonitoringProviderDetailsDisconnectTest(APITestCase): + endpoint = "sentry-api-0-organization-monitoring-provider-details" + method = "delete" + + def setUp(self) -> None: + super().setUp() + self.login_as(self.user) + + def test_disconnect_requires_feature_flag(self) -> None: + response = self.get_response(self.organization.slug, "gcp") + assert response.status_code == 404 + + def test_disconnect_deletes_identity_datadog(self) -> None: + idp = self.create_identity_provider(type="datadog", external_id="datadoghq.com") + self.create_identity( + user=self.user, + identity_provider=idp, + external_id="dd-user-123", + data={"access_token": "token"}, + ) + + with self.feature("organizations:seer-infra-telemetry"): + response = self.get_response(self.organization.slug, "datadog") + + assert response.status_code == 204 + assert not Identity.objects.filter(idp=idp, user=self.user).exists() + + def test_disconnect_deletes_identity_gcp(self) -> None: + idp = self.create_identity_provider(type="gcp") + self.create_identity( + user=self.user, + identity_provider=idp, + external_id="google-user-123", + data={"access_token": "token"}, + ) + + with self.feature("organizations:seer-infra-telemetry"): + response = self.get_response(self.organization.slug, "gcp") + + assert response.status_code == 204 + assert not Identity.objects.filter(idp=idp, user=self.user).exists() + + def test_disconnect_only_affects_requesting_user(self) -> None: + other_user = self.create_user() + self.create_member(organization=self.organization, user=other_user) + + idp = self.create_identity_provider(type="gcp") + self.create_identity( + user=self.user, + identity_provider=idp, + external_id="google-user-123", + data={"access_token": "token-a"}, + ) + other_identity = self.create_identity( + user=other_user, + identity_provider=idp, + external_id="google-user-456", + data={"access_token": "token-b"}, + ) + + with self.feature("organizations:seer-infra-telemetry"): + response = self.get_response(self.organization.slug, "gcp") + + assert response.status_code == 204 + assert not Identity.objects.filter(idp=idp, user=self.user).exists() + assert Identity.objects.filter(id=other_identity.id).exists() + + def test_disconnect_unknown_provider(self) -> None: + with self.feature("organizations:seer-infra-telemetry"): + response = self.get_response(self.organization.slug, "unknown") + + assert response.status_code == 400 + + def test_disconnect_not_connected(self) -> None: + with self.feature("organizations:seer-infra-telemetry"): + response = self.get_response(self.organization.slug, "gcp") + + assert response.status_code == 404 From bbed6c0d3781c9957bfdfa7480f9ea54dd46b92f Mon Sep 17 00:00:00 2001 From: "getsantry[bot]" <66042841+getsantry[bot]@users.noreply.github.com> Date: Thu, 11 Jun 2026 21:58:02 +0000 Subject: [PATCH 02/12] :hammer_and_wrench: Sync API Urls to TypeScript --- static/app/utils/api/knownSentryApiUrls.generated.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/static/app/utils/api/knownSentryApiUrls.generated.ts b/static/app/utils/api/knownSentryApiUrls.generated.ts index df8b92ca4fb83e..5f1acd86c72efa 100644 --- a/static/app/utils/api/knownSentryApiUrls.generated.ts +++ b/static/app/utils/api/knownSentryApiUrls.generated.ts @@ -271,6 +271,8 @@ export type KnownSentryApiUrls = | '/organizations/$organizationIdOrSlug/metrics-estimation-stats/' | '/organizations/$organizationIdOrSlug/metrics/data/' | '/organizations/$organizationIdOrSlug/missing-members/' + | '/organizations/$organizationIdOrSlug/monitoring-providers/' + | '/organizations/$organizationIdOrSlug/monitoring-providers/$providerKey/' | '/organizations/$organizationIdOrSlug/monitors-count/' | '/organizations/$organizationIdOrSlug/monitors-schedule-buckets/' | '/organizations/$organizationIdOrSlug/monitors-schedule-data/' From e01cffa0e9b7d1a2b43b3f79e7347be8c23dca93 Mon Sep 17 00:00:00 2001 From: Shashank Jarmale Date: Thu, 11 Jun 2026 15:15:24 -0700 Subject: [PATCH 03/12] Fix typing --- .../organization_monitoring_providers.py | 16 ++++++++++++---- .../test_organization_monitoring_providers.py | 10 ++++++---- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/sentry/api/endpoints/organization_monitoring_providers.py b/src/sentry/api/endpoints/organization_monitoring_providers.py index 567250c24dadeb..be4177f307e99a 100644 --- a/src/sentry/api/endpoints/organization_monitoring_providers.py +++ b/src/sentry/api/endpoints/organization_monitoring_providers.py @@ -25,7 +25,7 @@ MONITORING_PROVIDER_FEATURE = "organizations:seer-infra-telemetry" -def _get_pipeline_config(provider_key: str, request_data: dict) -> dict: +def _get_pipeline_config(provider_key: str, request_data: dict[str, str]) -> dict[str, str]: """Build provider-specific pipeline config from request body.""" config: dict[str, str] = {} if provider_key == "datadog": @@ -36,7 +36,7 @@ def _get_pipeline_config(provider_key: str, request_data: dict) -> dict: return config -def _get_identity_provider(provider_key: str, config: dict) -> IdentityProvider: +def _get_identity_provider(provider_key: str, config: dict[str, str]) -> IdentityProvider: """ Get or create the IdentityProvider for a monitoring provider. @@ -62,11 +62,15 @@ def get(self, request: Request, organization: RpcOrganization, **kwargs: object) if not features.has(MONITORING_PROVIDER_FEATURE, organization, actor=request.user): return Response(status=404) + user_id = request.user.id + if user_id is None: + return Response(status=401) + connected_identities = { identity.idp.type: identity for identity in Identity.objects.filter( idp__type__in=MONITORING_PROVIDERS.keys(), - user_id=request.user.id, + user_id=user_id, ).select_related("idp") } @@ -140,9 +144,13 @@ def delete( if provider_key not in MONITORING_PROVIDERS: return Response({"detail": "Unknown monitoring provider."}, status=400) + user_id = request.user.id + if user_id is None: + return Response(status=401) + deleted, _ = Identity.objects.filter( idp__type=provider_key, - user_id=request.user.id, + user_id=user_id, ).delete() if not deleted: diff --git a/tests/sentry/api/endpoints/test_organization_monitoring_providers.py b/tests/sentry/api/endpoints/test_organization_monitoring_providers.py index abc05e7aab03e5..9a389fe9c247d0 100644 --- a/tests/sentry/api/endpoints/test_organization_monitoring_providers.py +++ b/tests/sentry/api/endpoints/test_organization_monitoring_providers.py @@ -1,6 +1,6 @@ from __future__ import annotations -from unittest.mock import patch +from unittest.mock import MagicMock, patch from django.http import HttpResponseRedirect @@ -110,7 +110,9 @@ def test_connect_requires_feature_flag(self) -> None: @patch("sentry.identity.pipeline.IdentityPipeline.current_step") @patch("sentry.identity.pipeline.IdentityPipeline.initialize") - def test_connect_returns_redirect_url(self, mock_initialize, mock_current_step) -> None: + def test_connect_returns_redirect_url( + self, mock_initialize: MagicMock, mock_current_step: MagicMock + ) -> None: mock_current_step.return_value = HttpResponseRedirect( "https://accounts.google.com/o/oauth2/v2/auth?client_id=test" ) @@ -125,7 +127,7 @@ def test_connect_returns_redirect_url(self, mock_initialize, mock_current_step) @patch("sentry.identity.pipeline.IdentityPipeline.current_step") @patch("sentry.identity.pipeline.IdentityPipeline.initialize") def test_connect_gcp_creates_identity_provider( - self, mock_initialize, mock_current_step + self, mock_initialize: MagicMock, mock_current_step: MagicMock ) -> None: mock_current_step.return_value = HttpResponseRedirect("https://accounts.google.com/") @@ -143,7 +145,7 @@ def test_connect_gcp_creates_identity_provider( return_value=None, ) def test_connect_datadog_creates_identity_provider( - self, mock_init, mock_initialize, mock_current_step + self, mock_init: MagicMock, mock_initialize: MagicMock, mock_current_step: MagicMock ) -> None: mock_current_step.return_value = HttpResponseRedirect( "https://mcp.datadoghq.com/api/unstable/mcp-server/authorize" From aed29967066f361a462698d4d399e387e5a6ec05 Mon Sep 17 00:00:00 2001 From: Shashank Jarmale Date: Thu, 11 Jun 2026 15:29:42 -0700 Subject: [PATCH 04/12] Fixes / cleanup --- .../organization_monitoring_providers.py | 23 +++++++++++-------- .../test_organization_monitoring_providers.py | 2 +- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/sentry/api/endpoints/organization_monitoring_providers.py b/src/sentry/api/endpoints/organization_monitoring_providers.py index be4177f307e99a..0e56a6004ee8ea 100644 --- a/src/sentry/api/endpoints/organization_monitoring_providers.py +++ b/src/sentry/api/endpoints/organization_monitoring_providers.py @@ -43,9 +43,9 @@ def _get_identity_provider(provider_key: str, config: dict[str, str]) -> Identit Datadog uses per-site providers (external_id=site). GCP uses a single global provider. """ if provider_key == "datadog": - external_id = config.get("site") + external_id = config.get("site", "") else: - external_id = None + external_id = "" idp, _ = IdentityProvider.objects.get_or_create(type=provider_key, external_id=external_id) return idp @@ -110,8 +110,8 @@ def post( try: config = _get_pipeline_config(provider_key, request.data) - except ValueError as e: - return Response({"detail": str(e)}, status=400) + except ValueError: + return Response({"detail": "Invalid provider configuration."}, status=400) idp = _get_identity_provider(provider_key, config) @@ -148,12 +148,17 @@ def delete( if user_id is None: return Response(status=401) - deleted, _ = Identity.objects.filter( - idp__type=provider_key, - user_id=user_id, - ).delete() + identities = list( + Identity.objects.filter( + idp__type=provider_key, + user_id=user_id, + ) + ) - if not deleted: + if not identities: return Response({"detail": "Not connected to this provider."}, status=404) + for identity in identities: + identity.delete() + return Response(status=204) diff --git a/tests/sentry/api/endpoints/test_organization_monitoring_providers.py b/tests/sentry/api/endpoints/test_organization_monitoring_providers.py index 9a389fe9c247d0..973053acdeed44 100644 --- a/tests/sentry/api/endpoints/test_organization_monitoring_providers.py +++ b/tests/sentry/api/endpoints/test_organization_monitoring_providers.py @@ -166,7 +166,7 @@ def test_connect_datadog_requires_site(self) -> None: response = self.get_response(self.organization.slug, "datadog") assert response.status_code == 400 - assert "site" in response.data["detail"] + assert "Invalid provider configuration" in response.data["detail"] def test_connect_unknown_provider(self) -> None: with self.feature("organizations:seer-infra-telemetry"): From f08d22a0871b8e8dab33fd98d0e13904a5da43a6 Mon Sep 17 00:00:00 2001 From: Shashank Jarmale Date: Thu, 11 Jun 2026 15:48:15 -0700 Subject: [PATCH 05/12] More fixes --- .../organization_monitoring_providers.py | 32 ++++++++++++------- .../test_organization_monitoring_providers.py | 27 +++------------- 2 files changed, 26 insertions(+), 33 deletions(-) diff --git a/src/sentry/api/endpoints/organization_monitoring_providers.py b/src/sentry/api/endpoints/organization_monitoring_providers.py index 0e56a6004ee8ea..e3d4ca447e224b 100644 --- a/src/sentry/api/endpoints/organization_monitoring_providers.py +++ b/src/sentry/api/endpoints/organization_monitoring_providers.py @@ -10,7 +10,10 @@ from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import control_silo_endpoint -from sentry.api.bases.organization import ControlSiloOrganizationEndpoint +from sentry.api.bases.organization import ( + ControlSiloOrganizationEndpoint, + OrganizationPermission, +) from sentry.identity.pipeline import IdentityPipeline from sentry.organizations.services.organization.model import RpcOrganization from sentry.users.models.identity import Identity, IdentityProvider @@ -51,12 +54,21 @@ def _get_identity_provider(provider_key: str, config: dict[str, str]) -> Identit return idp +class MonitoringProviderPermission(OrganizationPermission): + scope_map = { + "GET": ["org:read", "org:write", "org:admin"], + "POST": ["org:write", "org:admin"], + "DELETE": ["org:write", "org:admin"], + } + + @control_silo_endpoint class OrganizationMonitoringProviderIndexEndpoint(ControlSiloOrganizationEndpoint): owner = ApiOwner.CODING_WORKFLOWS publish_status = { "GET": ApiPublishStatus.PRIVATE, } + permission_classes = (MonitoringProviderPermission,) def get(self, request: Request, organization: RpcOrganization, **kwargs: object) -> Response: if not features.has(MONITORING_PROVIDER_FEATURE, organization, actor=request.user): @@ -77,16 +89,13 @@ def get(self, request: Request, organization: RpcOrganization, **kwargs: object) providers = [] for key, meta in MONITORING_PROVIDERS.items(): identity = connected_identities.get(key) - entry: dict[str, str | bool] = { - "provider": key, - "name": meta["name"], - "connected": identity is not None, - } - if identity is not None: - email = identity.data.get("email") if identity.data else None - if email: - entry["email"] = email - providers.append(entry) + providers.append( + { + "provider": key, + "name": meta["name"], + "connected": identity is not None, + } + ) return Response({"providers": providers}) @@ -98,6 +107,7 @@ class OrganizationMonitoringProviderDetailsEndpoint(ControlSiloOrganizationEndpo "POST": ApiPublishStatus.PRIVATE, "DELETE": ApiPublishStatus.PRIVATE, } + permission_classes = (MonitoringProviderPermission,) def post( self, request: Request, organization: RpcOrganization, provider_key: str, **kwargs: object diff --git a/tests/sentry/api/endpoints/test_organization_monitoring_providers.py b/tests/sentry/api/endpoints/test_organization_monitoring_providers.py index 973053acdeed44..561439413d240d 100644 --- a/tests/sentry/api/endpoints/test_organization_monitoring_providers.py +++ b/tests/sentry/api/endpoints/test_organization_monitoring_providers.py @@ -40,7 +40,7 @@ def test_list_shows_connected_provider(self) -> None: user=self.user, identity_provider=idp, external_id="google-user-123", - data={"email": "user@example.com", "access_token": "token"}, + data={"access_token": "token"}, ) with self.feature("organizations:seer-infra-telemetry"): @@ -51,7 +51,6 @@ def test_list_shows_connected_provider(self) -> None: "provider": "gcp", "name": "Google Cloud Platform", "connected": True, - "email": "user@example.com", } assert providers["datadog"] == { "provider": "datadog", @@ -59,22 +58,6 @@ def test_list_shows_connected_provider(self) -> None: "connected": False, } - def test_list_connected_without_email(self) -> None: - idp = self.create_identity_provider(type="datadog", external_id="datadoghq.com") - self.create_identity( - user=self.user, - identity_provider=idp, - external_id="dd-user-123", - data={"access_token": "token"}, - ) - - with self.feature("organizations:seer-infra-telemetry"): - response = self.get_success_response(self.organization.slug) - - providers = {p["provider"]: p for p in response.data["providers"]} - assert providers["datadog"]["connected"] is True - assert "email" not in providers["datadog"] - def test_list_does_not_show_other_users_connections(self) -> None: other_user = self.create_user() self.create_member(organization=self.organization, user=other_user) @@ -108,8 +91,8 @@ def test_connect_requires_feature_flag(self) -> None: response = self.get_response(self.organization.slug, "datadog") assert response.status_code == 404 - @patch("sentry.identity.pipeline.IdentityPipeline.current_step") - @patch("sentry.identity.pipeline.IdentityPipeline.initialize") + @patch("sentry.api.endpoints.organization_monitoring_providers.IdentityPipeline.current_step") + @patch("sentry.api.endpoints.organization_monitoring_providers.IdentityPipeline.initialize") def test_connect_returns_redirect_url( self, mock_initialize: MagicMock, mock_current_step: MagicMock ) -> None: @@ -124,8 +107,8 @@ def test_connect_returns_redirect_url( assert response.data["redirectUrl"].startswith("https://accounts.google.com/") mock_initialize.assert_called_once() - @patch("sentry.identity.pipeline.IdentityPipeline.current_step") - @patch("sentry.identity.pipeline.IdentityPipeline.initialize") + @patch("sentry.api.endpoints.organization_monitoring_providers.IdentityPipeline.current_step") + @patch("sentry.api.endpoints.organization_monitoring_providers.IdentityPipeline.initialize") def test_connect_gcp_creates_identity_provider( self, mock_initialize: MagicMock, mock_current_step: MagicMock ) -> None: From 41c47393fb7884849acb73012cfa038323250341 Mon Sep 17 00:00:00 2001 From: Shashank Jarmale Date: Fri, 12 Jun 2026 09:25:22 -0700 Subject: [PATCH 06/12] Add Datadog site allowlist to identity provider --- .../organization_monitoring_providers.py | 17 +++++++------ src/sentry/identity/datadog/provider.py | 24 +++++++++++++++---- .../test_organization_monitoring_providers.py | 10 ++++++++ .../sentry/identity/datadog/test_provider.py | 8 ++++++- 4 files changed, 45 insertions(+), 14 deletions(-) diff --git a/src/sentry/api/endpoints/organization_monitoring_providers.py b/src/sentry/api/endpoints/organization_monitoring_providers.py index e3d4ca447e224b..34c08916c10883 100644 --- a/src/sentry/api/endpoints/organization_monitoring_providers.py +++ b/src/sentry/api/endpoints/organization_monitoring_providers.py @@ -120,18 +120,17 @@ def post( try: config = _get_pipeline_config(provider_key, request.data) + idp = _get_identity_provider(provider_key, config) + pipeline = IdentityPipeline( + request=request._request, + provider_key=provider_key, + organization=organization, + provider_model=idp, + config=config, + ) except ValueError: return Response({"detail": "Invalid provider configuration."}, status=400) - idp = _get_identity_provider(provider_key, config) - - pipeline = IdentityPipeline( - request=request._request, - provider_key=provider_key, - organization=organization, - provider_model=idp, - config=config, - ) pipeline.initialize() response = pipeline.current_step() diff --git a/src/sentry/identity/datadog/provider.py b/src/sentry/identity/datadog/provider.py index 6f9caef6a7798c..81014d7a437314 100644 --- a/src/sentry/identity/datadog/provider.py +++ b/src/sentry/identity/datadog/provider.py @@ -28,6 +28,19 @@ from sentry.users.models.identity import Identity from sentry.utils.http import absolute_uri +DATADOG_VALID_SITES = frozenset( + { + "datadoghq.com", + "us3.datadoghq.com", + "us5.datadoghq.com", + "datadoghq.eu", + "ddog-gov.com", + "us2.ddog-gov.com", + "ap1.datadoghq.com", + "ap2.datadoghq.com", + } +) + MCP_REGISTER_PATH = "/api/unstable/mcp-server/register" MCP_AUTHORIZE_PATH = "/api/unstable/mcp-server/authorize" MCP_TOKEN_PATH = "/api/unstable/mcp-server/token" @@ -38,9 +51,9 @@ def _basic_auth_header(client_id: str, client_secret: str) -> str: return "Basic " + base64.b64encode(f"{client_id}:{client_secret}".encode()).decode("ascii") -def get_user_info(access_token: str, site: str) -> dict[str, Any]: +def get_user_info(access_token: str, mcp_base_url: str) -> dict[str, Any]: """Fetch the current Datadog user via the MCP ``datadog://mcp/whoami`` resource.""" - url = f"https://mcp.{site}{MCP_ENDPOINT_PATH}" + url = f"{mcp_base_url}{MCP_ENDPOINT_PATH}" headers = {"Authorization": f"Bearer {access_token}", "Content-Type": "application/json"} init_resp = safe_urlopen( @@ -260,7 +273,10 @@ class DatadogIdentityProvider(OAuth2Provider): ) def _get_mcp_base_url(self) -> str: - return f"https://mcp.{self._get_oauth_parameter('site')}" + site = self._get_oauth_parameter("site") + if site not in DATADOG_VALID_SITES: + raise ValueError(f"Invalid Datadog site: {site}") + return f"https://mcp.{site}" def get_oauth_authorize_url(self) -> str: return self._get_mcp_base_url() + MCP_AUTHORIZE_PATH @@ -295,7 +311,7 @@ def build_identity(self, data: dict[str, Any]) -> dict[str, Any]: if not access_token: raise ValueError("Datadog token exchange did not return an access_token") - user = get_user_info(access_token, self._get_oauth_parameter("site")) + user = get_user_info(access_token, self._get_mcp_base_url()) oauth_data = self.get_oauth_data(token_data) diff --git a/tests/sentry/api/endpoints/test_organization_monitoring_providers.py b/tests/sentry/api/endpoints/test_organization_monitoring_providers.py index 561439413d240d..58315b3e0dc2b6 100644 --- a/tests/sentry/api/endpoints/test_organization_monitoring_providers.py +++ b/tests/sentry/api/endpoints/test_organization_monitoring_providers.py @@ -151,11 +151,19 @@ def test_connect_datadog_requires_site(self) -> None: assert response.status_code == 400 assert "Invalid provider configuration" in response.data["detail"] + def test_connect_datadog_invalid_site(self) -> None: + with self.feature("organizations:seer-infra-telemetry"): + response = self.get_response(self.organization.slug, "datadog", site="evil.example.com") + + assert response.status_code == 400 + assert "Invalid provider configuration" in response.data["detail"] + def test_connect_unknown_provider(self) -> None: with self.feature("organizations:seer-infra-telemetry"): response = self.get_response(self.organization.slug, "unknown") assert response.status_code == 400 + assert "Unknown monitoring provider" in response.data["detail"] @control_silo_test @@ -231,9 +239,11 @@ def test_disconnect_unknown_provider(self) -> None: response = self.get_response(self.organization.slug, "unknown") assert response.status_code == 400 + assert "Unknown monitoring provider" in response.data["detail"] def test_disconnect_not_connected(self) -> None: with self.feature("organizations:seer-infra-telemetry"): response = self.get_response(self.organization.slug, "gcp") assert response.status_code == 404 + assert "Not connected to this provider" in response.data["detail"] diff --git a/tests/sentry/identity/datadog/test_provider.py b/tests/sentry/identity/datadog/test_provider.py index e84c96863c76bb..a3ba5c506b2ee5 100644 --- a/tests/sentry/identity/datadog/test_provider.py +++ b/tests/sentry/identity/datadog/test_provider.py @@ -490,7 +490,7 @@ def test_build_identity(self, mock_get_user_info: MagicMock) -> None: assert result["data"]["client_id"] == "dcr-client-id" assert result["data"]["client_secret"] == "dcr-client-secret" assert result["data"]["site"] == "datadoghq.com" - mock_get_user_info.assert_called_once_with("token-abc", "datadoghq.com") + mock_get_user_info.assert_called_once_with("token-abc", "https://mcp.datadoghq.com") @patch("sentry.identity.datadog.provider.get_user_info") def test_build_identity_missing_access_token(self, mock_get_user_info: MagicMock) -> None: @@ -610,3 +610,9 @@ def test_refresh_identity_missing_refresh_token(self) -> None: with pytest.raises(IdentityNotValid, match="Missing refresh token"): self.provider.refresh_identity(identity) + + def test_invalid_site_rejected(self) -> None: + self.provider.config = {"site": "evil.example.com"} + + with pytest.raises(ValueError, match="Invalid Datadog site"): + self.provider._get_mcp_base_url() From d852e403371b94a58d04cc94bec84f23c7ae7a78 Mon Sep 17 00:00:00 2001 From: Shashank Jarmale Date: Mon, 15 Jun 2026 15:32:46 -0700 Subject: [PATCH 07/12] Update endpoint implementation with new identity provider behavior --- .../organization_monitoring_providers.py | 46 ++++++------------- .../test_organization_monitoring_providers.py | 10 ++-- 2 files changed, 18 insertions(+), 38 deletions(-) diff --git a/src/sentry/api/endpoints/organization_monitoring_providers.py b/src/sentry/api/endpoints/organization_monitoring_providers.py index e3d4ca447e224b..83c5ec95ad8031 100644 --- a/src/sentry/api/endpoints/organization_monitoring_providers.py +++ b/src/sentry/api/endpoints/organization_monitoring_providers.py @@ -28,32 +28,6 @@ MONITORING_PROVIDER_FEATURE = "organizations:seer-infra-telemetry" -def _get_pipeline_config(provider_key: str, request_data: dict[str, str]) -> dict[str, str]: - """Build provider-specific pipeline config from request body.""" - config: dict[str, str] = {} - if provider_key == "datadog": - site = request_data.get("site") - if not site: - raise ValueError("Datadog requires a 'site' parameter (e.g. 'datadoghq.com').") - config["site"] = site - return config - - -def _get_identity_provider(provider_key: str, config: dict[str, str]) -> IdentityProvider: - """ - Get or create the IdentityProvider for a monitoring provider. - - Datadog uses per-site providers (external_id=site). GCP uses a single global provider. - """ - if provider_key == "datadog": - external_id = config.get("site", "") - else: - external_id = "" - - idp, _ = IdentityProvider.objects.get_or_create(type=provider_key, external_id=external_id) - return idp - - class MonitoringProviderPermission(OrganizationPermission): scope_map = { "GET": ["org:read", "org:write", "org:admin"], @@ -118,12 +92,20 @@ def post( if provider_key not in MONITORING_PROVIDERS: return Response({"detail": "Unknown monitoring provider."}, status=400) - try: - config = _get_pipeline_config(provider_key, request.data) - except ValueError: - return Response({"detail": "Invalid provider configuration."}, status=400) - - idp = _get_identity_provider(provider_key, config) + config: dict[str, str] = {} + if provider_key == "datadog": + site = request.data.get("site") + if not site: + return Response( + {"detail": "Datadog requires a 'site' parameter (e.g. 'datadoghq.com')."}, + status=400, + ) + config["site"] = site + + # Datadog: the IdentityProvider is auto-created during the pipeline + idp: IdentityProvider | None = None + if provider_key != "datadog": + idp, _ = IdentityProvider.objects.get_or_create(type=provider_key, external_id="") pipeline = IdentityPipeline( request=request._request, diff --git a/tests/sentry/api/endpoints/test_organization_monitoring_providers.py b/tests/sentry/api/endpoints/test_organization_monitoring_providers.py index 561439413d240d..8030af0770a556 100644 --- a/tests/sentry/api/endpoints/test_organization_monitoring_providers.py +++ b/tests/sentry/api/endpoints/test_organization_monitoring_providers.py @@ -127,21 +127,19 @@ def test_connect_gcp_creates_identity_provider( "sentry.api.endpoints.organization_monitoring_providers.IdentityPipeline.__init__", return_value=None, ) - def test_connect_datadog_creates_identity_provider( + def test_connect_datadog_does_not_create_identity_provider( self, mock_init: MagicMock, mock_initialize: MagicMock, mock_current_step: MagicMock ) -> None: mock_current_step.return_value = HttpResponseRedirect( "https://mcp.datadoghq.com/api/unstable/mcp-server/authorize" ) - assert not IdentityProvider.objects.filter(type="datadog").exists() - with self.feature("organizations:seer-infra-telemetry"): response = self.get_success_response( self.organization.slug, "datadog", site="datadoghq.com" ) - assert IdentityProvider.objects.filter(type="datadog", external_id="datadoghq.com").exists() + assert not IdentityProvider.objects.filter(type="datadog").exists() assert response.data["redirectUrl"].startswith("https://mcp.datadoghq.com/") def test_connect_datadog_requires_site(self) -> None: @@ -149,7 +147,7 @@ def test_connect_datadog_requires_site(self) -> None: response = self.get_response(self.organization.slug, "datadog") assert response.status_code == 400 - assert "Invalid provider configuration" in response.data["detail"] + assert "site" in response.data["detail"] def test_connect_unknown_provider(self) -> None: with self.feature("organizations:seer-infra-telemetry"): @@ -172,7 +170,7 @@ def test_disconnect_requires_feature_flag(self) -> None: assert response.status_code == 404 def test_disconnect_deletes_identity_datadog(self) -> None: - idp = self.create_identity_provider(type="datadog", external_id="datadoghq.com") + idp = self.create_identity_provider(type="datadog", external_id="dd-org-456") self.create_identity( user=self.user, identity_provider=idp, From 2d63fb11b5ca7892c62939cf328be7ad46c855f3 Mon Sep 17 00:00:00 2001 From: Shashank Jarmale Date: Mon, 15 Jun 2026 15:46:02 -0700 Subject: [PATCH 08/12] Make request validation consistent --- .../api/endpoints/organization_monitoring_providers.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/sentry/api/endpoints/organization_monitoring_providers.py b/src/sentry/api/endpoints/organization_monitoring_providers.py index 83c5ec95ad8031..28bd658d8f92ed 100644 --- a/src/sentry/api/endpoints/organization_monitoring_providers.py +++ b/src/sentry/api/endpoints/organization_monitoring_providers.py @@ -89,6 +89,9 @@ def post( if not features.has(MONITORING_PROVIDER_FEATURE, organization, actor=request.user): return Response(status=404) + if request.user.id is None: + return Response(status=401) + if provider_key not in MONITORING_PROVIDERS: return Response({"detail": "Unknown monitoring provider."}, status=400) @@ -133,13 +136,13 @@ def delete( if not features.has(MONITORING_PROVIDER_FEATURE, organization, actor=request.user): return Response(status=404) - if provider_key not in MONITORING_PROVIDERS: - return Response({"detail": "Unknown monitoring provider."}, status=400) - user_id = request.user.id if user_id is None: return Response(status=401) + if provider_key not in MONITORING_PROVIDERS: + return Response({"detail": "Unknown monitoring provider."}, status=400) + identities = list( Identity.objects.filter( idp__type=provider_key, From 73582f83988a3a90e95318b47367998f8bd4f7d6 Mon Sep 17 00:00:00 2001 From: Shashank Jarmale Date: Mon, 15 Jun 2026 16:00:12 -0700 Subject: [PATCH 09/12] Fixes --- .../organization_monitoring_providers.py | 18 +++++++++++------- src/sentry/identity/datadog/provider.py | 1 + 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/sentry/api/endpoints/organization_monitoring_providers.py b/src/sentry/api/endpoints/organization_monitoring_providers.py index 28bd658d8f92ed..4abcc78b504915 100644 --- a/src/sentry/api/endpoints/organization_monitoring_providers.py +++ b/src/sentry/api/endpoints/organization_monitoring_providers.py @@ -110,13 +110,17 @@ def post( if provider_key != "datadog": idp, _ = IdentityProvider.objects.get_or_create(type=provider_key, external_id="") - pipeline = IdentityPipeline( - request=request._request, - provider_key=provider_key, - organization=organization, - provider_model=idp, - config=config, - ) + try: + pipeline = IdentityPipeline( + request=request._request, + provider_key=provider_key, + organization=organization, + provider_model=idp, + config=config, + ) + except ValueError: + return Response({"detail": "Invalid provider configuration."}, status=400) + pipeline.initialize() response = pipeline.current_step() diff --git a/src/sentry/identity/datadog/provider.py b/src/sentry/identity/datadog/provider.py index 66bad030e4a391..8fb534aea6f5b5 100644 --- a/src/sentry/identity/datadog/provider.py +++ b/src/sentry/identity/datadog/provider.py @@ -318,6 +318,7 @@ def build_identity(self, data: dict[str, Any]) -> dict[str, Any]: "User info response missing required fields (user_uuid, org_uuid)" ) + site = self._get_oauth_parameter("site") oauth_data = self.get_oauth_data(token_data) # Persist DCR credentials and site so refresh_identity can access them outside a pipeline context. From 0b62de2aa74b1034dcdee1fc1c8fad70849c83b9 Mon Sep 17 00:00:00 2001 From: Shashank Jarmale Date: Mon, 15 Jun 2026 16:13:32 -0700 Subject: [PATCH 10/12] Cleanup --- .../organization_monitoring_providers.py | 21 +++++++++---------- .../test_organization_monitoring_providers.py | 2 +- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/sentry/api/endpoints/organization_monitoring_providers.py b/src/sentry/api/endpoints/organization_monitoring_providers.py index 4abcc78b504915..885719dc41e39c 100644 --- a/src/sentry/api/endpoints/organization_monitoring_providers.py +++ b/src/sentry/api/endpoints/organization_monitoring_providers.py @@ -14,6 +14,7 @@ ControlSiloOrganizationEndpoint, OrganizationPermission, ) +from sentry.identity.datadog.provider import DATADOG_VALID_SITES from sentry.identity.pipeline import IdentityPipeline from sentry.organizations.services.organization.model import RpcOrganization from sentry.users.models.identity import Identity, IdentityProvider @@ -103,6 +104,8 @@ def post( {"detail": "Datadog requires a 'site' parameter (e.g. 'datadoghq.com')."}, status=400, ) + elif site not in DATADOG_VALID_SITES: + return Response({"detail": f"Invalid Datadog site: {site}"}, status=400) config["site"] = site # Datadog: the IdentityProvider is auto-created during the pipeline @@ -110,17 +113,13 @@ def post( if provider_key != "datadog": idp, _ = IdentityProvider.objects.get_or_create(type=provider_key, external_id="") - try: - pipeline = IdentityPipeline( - request=request._request, - provider_key=provider_key, - organization=organization, - provider_model=idp, - config=config, - ) - except ValueError: - return Response({"detail": "Invalid provider configuration."}, status=400) - + pipeline = IdentityPipeline( + request=request._request, + provider_key=provider_key, + organization=organization, + provider_model=idp, + config=config, + ) pipeline.initialize() response = pipeline.current_step() diff --git a/tests/sentry/api/endpoints/test_organization_monitoring_providers.py b/tests/sentry/api/endpoints/test_organization_monitoring_providers.py index 5b5d08a7f6ea6c..8957b553040ab0 100644 --- a/tests/sentry/api/endpoints/test_organization_monitoring_providers.py +++ b/tests/sentry/api/endpoints/test_organization_monitoring_providers.py @@ -154,7 +154,7 @@ def test_connect_datadog_invalid_site(self) -> None: response = self.get_response(self.organization.slug, "datadog", site="evil.example.com") assert response.status_code == 400 - assert "Invalid provider configuration" in response.data["detail"] + assert "Invalid Datadog site" in response.data["detail"] def test_connect_unknown_provider(self) -> None: with self.feature("organizations:seer-infra-telemetry"): From 21d95c6eff4d0076b743a22cd83ac1eab980a20e Mon Sep 17 00:00:00 2001 From: Shashank Jarmale Date: Mon, 15 Jun 2026 16:21:03 -0700 Subject: [PATCH 11/12] Add validation to refresh identity flow --- src/sentry/identity/datadog/provider.py | 2 ++ tests/sentry/identity/datadog/test_provider.py | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/src/sentry/identity/datadog/provider.py b/src/sentry/identity/datadog/provider.py index 8fb534aea6f5b5..e8883356021955 100644 --- a/src/sentry/identity/datadog/provider.py +++ b/src/sentry/identity/datadog/provider.py @@ -365,6 +365,8 @@ def refresh_identity(self, identity: Identity | RpcIdentity, **kwargs: Any) -> N site = identity.data.get("site") if not site: raise IdentityNotValid("Missing Datadog site") + elif site not in DATADOG_VALID_SITES: + raise IdentityNotValid(f"Invalid Datadog site: {site}") self.config["site"] = site client_id = identity.data.get("client_id") diff --git a/tests/sentry/identity/datadog/test_provider.py b/tests/sentry/identity/datadog/test_provider.py index 2ea75d8ca78309..8f3f6ea7077d13 100644 --- a/tests/sentry/identity/datadog/test_provider.py +++ b/tests/sentry/identity/datadog/test_provider.py @@ -636,6 +636,12 @@ def test_refresh_identity_missing_site(self) -> None: with pytest.raises(IdentityNotValid, match="Missing Datadog site"): self.provider.refresh_identity(identity) + def test_refresh_identity_invalid_site(self) -> None: + identity = self._make_identity(site="evil.example.com") + + with pytest.raises(IdentityNotValid, match="Invalid Datadog site"): + self.provider.refresh_identity(identity) + def test_refresh_identity_missing_dcr_credentials(self) -> None: identity = self._make_identity(client_id=None, client_secret=None) From cecb5599e237bffcdd50a98ad277253e2b3a8004 Mon Sep 17 00:00:00 2001 From: Shashank Jarmale Date: Tue, 16 Jun 2026 13:28:30 -0700 Subject: [PATCH 12/12] Fix merge --- .../organization_monitoring_providers.py | 162 ------------ src/sentry/identity/datadog/provider.py | 2 + ...rganization_monitoring_provider_details.py | 10 + .../test_organization_monitoring_providers.py | 247 ------------------ 4 files changed, 12 insertions(+), 409 deletions(-) delete mode 100644 src/sentry/api/endpoints/organization_monitoring_providers.py delete mode 100644 tests/sentry/api/endpoints/test_organization_monitoring_providers.py diff --git a/src/sentry/api/endpoints/organization_monitoring_providers.py b/src/sentry/api/endpoints/organization_monitoring_providers.py deleted file mode 100644 index 885719dc41e39c..00000000000000 --- a/src/sentry/api/endpoints/organization_monitoring_providers.py +++ /dev/null @@ -1,162 +0,0 @@ -from __future__ import annotations - -import logging - -from django.http import HttpResponseRedirect -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 control_silo_endpoint -from sentry.api.bases.organization import ( - ControlSiloOrganizationEndpoint, - OrganizationPermission, -) -from sentry.identity.datadog.provider import DATADOG_VALID_SITES -from sentry.identity.pipeline import IdentityPipeline -from sentry.organizations.services.organization.model import RpcOrganization -from sentry.users.models.identity import Identity, IdentityProvider - -logger = logging.getLogger(__name__) - -MONITORING_PROVIDERS: dict[str, dict[str, str]] = { - "datadog": {"name": "Datadog"}, - "gcp": {"name": "Google Cloud Platform"}, -} - -MONITORING_PROVIDER_FEATURE = "organizations:seer-infra-telemetry" - - -class MonitoringProviderPermission(OrganizationPermission): - scope_map = { - "GET": ["org:read", "org:write", "org:admin"], - "POST": ["org:write", "org:admin"], - "DELETE": ["org:write", "org:admin"], - } - - -@control_silo_endpoint -class OrganizationMonitoringProviderIndexEndpoint(ControlSiloOrganizationEndpoint): - owner = ApiOwner.CODING_WORKFLOWS - publish_status = { - "GET": ApiPublishStatus.PRIVATE, - } - permission_classes = (MonitoringProviderPermission,) - - def get(self, request: Request, organization: RpcOrganization, **kwargs: object) -> Response: - if not features.has(MONITORING_PROVIDER_FEATURE, organization, actor=request.user): - return Response(status=404) - - user_id = request.user.id - if user_id is None: - return Response(status=401) - - connected_identities = { - identity.idp.type: identity - for identity in Identity.objects.filter( - idp__type__in=MONITORING_PROVIDERS.keys(), - user_id=user_id, - ).select_related("idp") - } - - providers = [] - for key, meta in MONITORING_PROVIDERS.items(): - identity = connected_identities.get(key) - providers.append( - { - "provider": key, - "name": meta["name"], - "connected": identity is not None, - } - ) - - return Response({"providers": providers}) - - -@control_silo_endpoint -class OrganizationMonitoringProviderDetailsEndpoint(ControlSiloOrganizationEndpoint): - owner = ApiOwner.CODING_WORKFLOWS - publish_status = { - "POST": ApiPublishStatus.PRIVATE, - "DELETE": ApiPublishStatus.PRIVATE, - } - permission_classes = (MonitoringProviderPermission,) - - def post( - self, request: Request, organization: RpcOrganization, provider_key: str, **kwargs: object - ) -> Response: - if not features.has(MONITORING_PROVIDER_FEATURE, organization, actor=request.user): - return Response(status=404) - - if request.user.id is None: - return Response(status=401) - - if provider_key not in MONITORING_PROVIDERS: - return Response({"detail": "Unknown monitoring provider."}, status=400) - - config: dict[str, str] = {} - if provider_key == "datadog": - site = request.data.get("site") - if not site: - return Response( - {"detail": "Datadog requires a 'site' parameter (e.g. 'datadoghq.com')."}, - status=400, - ) - elif site not in DATADOG_VALID_SITES: - return Response({"detail": f"Invalid Datadog site: {site}"}, status=400) - config["site"] = site - - # Datadog: the IdentityProvider is auto-created during the pipeline - idp: IdentityProvider | None = None - if provider_key != "datadog": - idp, _ = IdentityProvider.objects.get_or_create(type=provider_key, external_id="") - - pipeline = IdentityPipeline( - request=request._request, - provider_key=provider_key, - organization=organization, - provider_model=idp, - config=config, - ) - pipeline.initialize() - - response = pipeline.current_step() - - if isinstance(response, HttpResponseRedirect): - return Response({"redirectUrl": response.url}) - - logger.error( - "monitoring_provider.connect.unexpected_response", - extra={"provider": provider_key, "response_type": type(response).__name__}, - ) - return Response({"detail": "Failed to start OAuth flow."}, status=500) - - def delete( - self, request: Request, organization: RpcOrganization, provider_key: str, **kwargs: object - ) -> Response: - if not features.has(MONITORING_PROVIDER_FEATURE, organization, actor=request.user): - return Response(status=404) - - user_id = request.user.id - if user_id is None: - return Response(status=401) - - if provider_key not in MONITORING_PROVIDERS: - return Response({"detail": "Unknown monitoring provider."}, status=400) - - identities = list( - Identity.objects.filter( - idp__type=provider_key, - user_id=user_id, - ) - ) - - if not identities: - return Response({"detail": "Not connected to this provider."}, status=404) - - for identity in identities: - identity.delete() - - return Response(status=204) diff --git a/src/sentry/identity/datadog/provider.py b/src/sentry/identity/datadog/provider.py index 78b457f2c2e178..1aaab48772b66e 100644 --- a/src/sentry/identity/datadog/provider.py +++ b/src/sentry/identity/datadog/provider.py @@ -277,6 +277,8 @@ def get_pipeline_config(self, data: dict[str, Any]) -> dict[str, str]: site = data.get("site") if not site: raise ValueError("Datadog requires a 'site' parameter (e.g. 'datadoghq.com').") + elif site not in DATADOG_VALID_SITES: + raise ValueError(f"Invalid Datadog site: {site}") return {"site": site} def _get_mcp_base_url(self) -> str: diff --git a/tests/sentry/api/endpoints/test_organization_monitoring_provider_details.py b/tests/sentry/api/endpoints/test_organization_monitoring_provider_details.py index be5bc919ee8bea..970428495cb534 100644 --- a/tests/sentry/api/endpoints/test_organization_monitoring_provider_details.py +++ b/tests/sentry/api/endpoints/test_organization_monitoring_provider_details.py @@ -92,11 +92,19 @@ def test_connect_datadog_requires_site(self) -> None: assert response.status_code == 400 assert "site" in response.data["detail"] + def test_connect_datadog_invalid_site(self) -> None: + with self.feature("organizations:seer-infra-telemetry"): + response = self.get_response(self.organization.slug, "datadog", site="evil.example.com") + + assert response.status_code == 400 + assert "Invalid Datadog site" in response.data["detail"] + def test_connect_unknown_provider(self) -> None: with self.feature("organizations:seer-infra-telemetry"): response = self.get_response(self.organization.slug, "unknown") assert response.status_code == 400 + assert "Unknown monitoring provider" in response.data["detail"] @patch( "sentry.api.endpoints.organization_monitoring_provider_details.IdentityPipeline.current_step" @@ -200,12 +208,14 @@ def test_disconnect_unknown_provider(self) -> None: response = self.get_response(self.organization.slug, "unknown") assert response.status_code == 400 + assert "Unknown monitoring provider" in response.data["detail"] def test_disconnect_not_connected(self) -> None: with self.feature("organizations:seer-infra-telemetry"): response = self.get_response(self.organization.slug, "gcp") assert response.status_code == 404 + assert "Not connected to this provider" in response.data["detail"] def test_disconnect_allowed_for_org_read_member(self) -> None: member_user = self.create_user() diff --git a/tests/sentry/api/endpoints/test_organization_monitoring_providers.py b/tests/sentry/api/endpoints/test_organization_monitoring_providers.py deleted file mode 100644 index 8957b553040ab0..00000000000000 --- a/tests/sentry/api/endpoints/test_organization_monitoring_providers.py +++ /dev/null @@ -1,247 +0,0 @@ -from __future__ import annotations - -from unittest.mock import MagicMock, patch - -from django.http import HttpResponseRedirect - -from sentry.testutils.cases import APITestCase -from sentry.testutils.silo import control_silo_test -from sentry.users.models.identity import Identity, IdentityProvider - - -@control_silo_test -class OrganizationMonitoringProviderIndexEndpointTest(APITestCase): - endpoint = "sentry-api-0-organization-monitoring-providers" - method = "get" - - def setUp(self) -> None: - super().setUp() - self.login_as(self.user) - - def test_list_requires_feature_flag(self) -> None: - response = self.get_response(self.organization.slug) - assert response.status_code == 404 - - def test_list_providers(self) -> None: - with self.feature("organizations:seer-infra-telemetry"): - response = self.get_success_response(self.organization.slug) - - providers = {p["provider"]: p for p in response.data["providers"]} - assert "gcp" in providers - assert "datadog" in providers - assert providers["gcp"]["name"] == "Google Cloud Platform" - assert providers["datadog"]["name"] == "Datadog" - assert providers["gcp"]["connected"] is False - assert providers["datadog"]["connected"] is False - - def test_list_shows_connected_provider(self) -> None: - idp = self.create_identity_provider(type="gcp") - self.create_identity( - user=self.user, - identity_provider=idp, - external_id="google-user-123", - data={"access_token": "token"}, - ) - - with self.feature("organizations:seer-infra-telemetry"): - response = self.get_success_response(self.organization.slug) - - providers = {p["provider"]: p for p in response.data["providers"]} - assert providers["gcp"] == { - "provider": "gcp", - "name": "Google Cloud Platform", - "connected": True, - } - assert providers["datadog"] == { - "provider": "datadog", - "name": "Datadog", - "connected": False, - } - - def test_list_does_not_show_other_users_connections(self) -> None: - other_user = self.create_user() - self.create_member(organization=self.organization, user=other_user) - - idp = self.create_identity_provider(type="gcp") - self.create_identity( - user=other_user, - identity_provider=idp, - external_id="other-google-user-456", - data={"email": "other@example.com", "access_token": "token"}, - ) - - with self.feature("organizations:seer-infra-telemetry"): - response = self.get_success_response(self.organization.slug) - - providers = {p["provider"]: p for p in response.data["providers"]} - assert providers["gcp"]["connected"] is False - assert providers["datadog"]["connected"] is False - - -@control_silo_test -class OrganizationMonitoringProviderDetailsConnectTest(APITestCase): - endpoint = "sentry-api-0-organization-monitoring-provider-details" - method = "post" - - def setUp(self) -> None: - super().setUp() - self.login_as(self.user) - - def test_connect_requires_feature_flag(self) -> None: - response = self.get_response(self.organization.slug, "datadog") - assert response.status_code == 404 - - @patch("sentry.api.endpoints.organization_monitoring_providers.IdentityPipeline.current_step") - @patch("sentry.api.endpoints.organization_monitoring_providers.IdentityPipeline.initialize") - def test_connect_returns_redirect_url( - self, mock_initialize: MagicMock, mock_current_step: MagicMock - ) -> None: - mock_current_step.return_value = HttpResponseRedirect( - "https://accounts.google.com/o/oauth2/v2/auth?client_id=test" - ) - - with self.feature("organizations:seer-infra-telemetry"): - response = self.get_success_response(self.organization.slug, "gcp") - - assert "redirectUrl" in response.data - assert response.data["redirectUrl"].startswith("https://accounts.google.com/") - mock_initialize.assert_called_once() - - @patch("sentry.api.endpoints.organization_monitoring_providers.IdentityPipeline.current_step") - @patch("sentry.api.endpoints.organization_monitoring_providers.IdentityPipeline.initialize") - def test_connect_gcp_creates_identity_provider( - self, mock_initialize: MagicMock, mock_current_step: MagicMock - ) -> None: - mock_current_step.return_value = HttpResponseRedirect("https://accounts.google.com/") - - assert not IdentityProvider.objects.filter(type="gcp").exists() - - with self.feature("organizations:seer-infra-telemetry"): - self.get_success_response(self.organization.slug, "gcp") - - assert IdentityProvider.objects.filter(type="gcp").exists() - - @patch("sentry.api.endpoints.organization_monitoring_providers.IdentityPipeline.current_step") - @patch("sentry.api.endpoints.organization_monitoring_providers.IdentityPipeline.initialize") - @patch( - "sentry.api.endpoints.organization_monitoring_providers.IdentityPipeline.__init__", - return_value=None, - ) - def test_connect_datadog_does_not_create_identity_provider( - self, mock_init: MagicMock, mock_initialize: MagicMock, mock_current_step: MagicMock - ) -> None: - mock_current_step.return_value = HttpResponseRedirect( - "https://mcp.datadoghq.com/api/unstable/mcp-server/authorize" - ) - - with self.feature("organizations:seer-infra-telemetry"): - response = self.get_success_response( - self.organization.slug, "datadog", site="datadoghq.com" - ) - - assert not IdentityProvider.objects.filter(type="datadog").exists() - assert response.data["redirectUrl"].startswith("https://mcp.datadoghq.com/") - - def test_connect_datadog_requires_site(self) -> None: - with self.feature("organizations:seer-infra-telemetry"): - response = self.get_response(self.organization.slug, "datadog") - - assert response.status_code == 400 - assert "site" in response.data["detail"] - - def test_connect_datadog_invalid_site(self) -> None: - with self.feature("organizations:seer-infra-telemetry"): - response = self.get_response(self.organization.slug, "datadog", site="evil.example.com") - - assert response.status_code == 400 - assert "Invalid Datadog site" in response.data["detail"] - - def test_connect_unknown_provider(self) -> None: - with self.feature("organizations:seer-infra-telemetry"): - response = self.get_response(self.organization.slug, "unknown") - - assert response.status_code == 400 - assert "Unknown monitoring provider" in response.data["detail"] - - -@control_silo_test -class OrganizationMonitoringProviderDetailsDisconnectTest(APITestCase): - endpoint = "sentry-api-0-organization-monitoring-provider-details" - method = "delete" - - def setUp(self) -> None: - super().setUp() - self.login_as(self.user) - - def test_disconnect_requires_feature_flag(self) -> None: - response = self.get_response(self.organization.slug, "gcp") - assert response.status_code == 404 - - def test_disconnect_deletes_identity_datadog(self) -> None: - idp = self.create_identity_provider(type="datadog", external_id="dd-org-456") - self.create_identity( - user=self.user, - identity_provider=idp, - external_id="dd-user-123", - data={"access_token": "token"}, - ) - - with self.feature("organizations:seer-infra-telemetry"): - response = self.get_response(self.organization.slug, "datadog") - - assert response.status_code == 204 - assert not Identity.objects.filter(idp=idp, user=self.user).exists() - - def test_disconnect_deletes_identity_gcp(self) -> None: - idp = self.create_identity_provider(type="gcp") - self.create_identity( - user=self.user, - identity_provider=idp, - external_id="google-user-123", - data={"access_token": "token"}, - ) - - with self.feature("organizations:seer-infra-telemetry"): - response = self.get_response(self.organization.slug, "gcp") - - assert response.status_code == 204 - assert not Identity.objects.filter(idp=idp, user=self.user).exists() - - def test_disconnect_only_affects_requesting_user(self) -> None: - other_user = self.create_user() - self.create_member(organization=self.organization, user=other_user) - - idp = self.create_identity_provider(type="gcp") - self.create_identity( - user=self.user, - identity_provider=idp, - external_id="google-user-123", - data={"access_token": "token-a"}, - ) - other_identity = self.create_identity( - user=other_user, - identity_provider=idp, - external_id="google-user-456", - data={"access_token": "token-b"}, - ) - - with self.feature("organizations:seer-infra-telemetry"): - response = self.get_response(self.organization.slug, "gcp") - - assert response.status_code == 204 - assert not Identity.objects.filter(idp=idp, user=self.user).exists() - assert Identity.objects.filter(id=other_identity.id).exists() - - def test_disconnect_unknown_provider(self) -> None: - with self.feature("organizations:seer-infra-telemetry"): - response = self.get_response(self.organization.slug, "unknown") - - assert response.status_code == 400 - assert "Unknown monitoring provider" in response.data["detail"] - - def test_disconnect_not_connected(self) -> None: - with self.feature("organizations:seer-infra-telemetry"): - response = self.get_response(self.organization.slug, "gcp") - - assert response.status_code == 404 - assert "Not connected to this provider" in response.data["detail"]