From 2a9bc52f8e44dcdfb900cf7a75b1b2a61b24e3b8 Mon Sep 17 00:00:00 2001 From: Gabe Villalobos Date: Mon, 13 Apr 2026 11:34:41 -0700 Subject: [PATCH 1/5] Refactors JWT token validation to shared class for Atlassian Connect apps --- .../integrations/jira/webhooks/installed.py | 48 ++++--------------- .../integrations/utils/atlassian_connect.py | 38 ++++++++++++++- .../integrations/jira/test_installed.py | 25 +++++----- 3 files changed, 57 insertions(+), 54 deletions(-) diff --git a/src/sentry/integrations/jira/webhooks/installed.py b/src/sentry/integrations/jira/webhooks/installed.py index d43c478f913100..496afbf5665690 100644 --- a/src/sentry/integrations/jira/webhooks/installed.py +++ b/src/sentry/integrations/jira/webhooks/installed.py @@ -1,6 +1,5 @@ import sentry_sdk from django.db import router, transaction -from jwt import DecodeError, ExpiredSignatureError, InvalidKeyError, InvalidSignatureError from rest_framework import status from rest_framework.request import Request from rest_framework.response import Response @@ -14,12 +13,14 @@ from sentry.integrations.jira.webhooks.base import JiraWebhookBase from sentry.integrations.pipeline import ensure_integration from sentry.integrations.project_management.metrics import ProjectManagementFailuresReason -from sentry.integrations.utils.atlassian_connect import authenticate_asymmetric_jwt, verify_claims +from sentry.integrations.utils.atlassian_connect import ( + AtlassianConnectTokenValidator, + AtlassianConnectValidationError, +) from sentry.integrations.utils.metrics import ( IntegrationPipelineViewEvent, IntegrationPipelineViewType, ) -from sentry.utils import jwt @control_silo_endpoint @@ -38,57 +39,24 @@ def post(self, request: Request, *args, **kwargs) -> Response: domain=IntegrationDomain.PROJECT_MANAGEMENT, provider_key=self.provider, ).capture() as lifecycle: - token = self.get_token(request) state = request.data if not state: lifecycle.record_failure(ProjectManagementFailuresReason.INSTALLATION_STATE_MISSING) return self.respond(status=status.HTTP_400_BAD_REQUEST) - try: - key_id = jwt.peek_header(token).get("kid") - except DecodeError: - lifecycle.record_halt(halt_reason="Failed to fetch key id") - return self.respond( - {"detail": "Failed to fetch key id"}, status=status.HTTP_400_BAD_REQUEST - ) - lifecycle.add_extras( { - "key_id": key_id, "base_url": state.get("baseUrl", ""), "description": state.get("description", ""), "clientKey": state.get("clientKey", ""), } ) - if not key_id: - lifecycle.record_halt(halt_reason="Missing key_id (kid)") - return self.respond( - {"detail": "Missing key id"}, status=status.HTTP_400_BAD_REQUEST - ) try: - decoded_claims = authenticate_asymmetric_jwt(token, key_id) - verify_claims(decoded_claims, request.path, request.GET, method="POST") - except InvalidKeyError: - lifecycle.record_halt(halt_reason="JWT contained invalid key_id (kid)") - return self.respond( - {"detail": "Invalid key id"}, status=status.HTTP_400_BAD_REQUEST - ) - except ExpiredSignatureError as e: - lifecycle.record_failure(e) - return self.respond( - {"detail": "Expired signature"}, status=status.HTTP_400_BAD_REQUEST - ) - except InvalidSignatureError: - lifecycle.record_halt(halt_reason="JWT contained invalid signature") - return self.respond( - {"detail": "Invalid signature"}, status=status.HTTP_400_BAD_REQUEST - ) - except DecodeError: - lifecycle.record_halt(halt_reason="Could not decode JWT token") - return self.respond( - {"detail": "Could not decode JWT token"}, status=status.HTTP_400_BAD_REQUEST - ) + AtlassianConnectTokenValidator(request).get_token() + except AtlassianConnectValidationError as e: + lifecycle.record_halt(halt_reason=e.message) + return self.respond({"detail": e.message}, status=status.HTTP_400_BAD_REQUEST) data = JiraIntegrationProvider().build_integration(state) integration = ensure_integration(self.provider, data) diff --git a/src/sentry/integrations/utils/atlassian_connect.py b/src/sentry/integrations/utils/atlassian_connect.py index 509191f1e22856..30dff793481682 100644 --- a/src/sentry/integrations/utils/atlassian_connect.py +++ b/src/sentry/integrations/utils/atlassian_connect.py @@ -5,7 +5,7 @@ import requests from django.http import HttpRequest -from jwt import ExpiredSignatureError, InvalidSignatureError +from jwt import DecodeError, ExpiredSignatureError, InvalidKeyError, InvalidSignatureError from sentry.integrations.models.integration import Integration from sentry.integrations.services.integration.model import RpcIntegration @@ -16,7 +16,9 @@ class AtlassianConnectValidationError(Exception): - pass + def __init__(self, message: str) -> None: + self.message = message + super().__init__(message) def get_query_hash( @@ -152,3 +154,35 @@ def parse_integration_from_request(request: HttpRequest, provider: str) -> Integ method=request.method if request.method else "POST", ) return Integration.objects.filter(id=rpc_integration.id).first() + + +class AtlassianConnectTokenValidator: + def __init__(self, request: HttpRequest) -> None: + self.request = request + + def get_token(self) -> str: + try: + token = get_token(self.request) + except Exception: + raise AtlassianConnectValidationError("Failed to retrieve token from request headers") + self._validate_token(token) + return token + + def _validate_token(self, token: str) -> None: + try: + key_id = jwt.peek_header(token).get("kid") + except DecodeError: + raise AtlassianConnectValidationError("Failed to fetch key_id (kid)") + if not key_id: + raise AtlassianConnectValidationError("Missing key_id (kid)") + try: + decoded_claims = authenticate_asymmetric_jwt(token, key_id) + verify_claims(decoded_claims, self.request.path, self.request.GET, self.request.method) + except InvalidKeyError: + raise AtlassianConnectValidationError("JWT contained invalid key_id (kid)") + except ExpiredSignatureError: + raise AtlassianConnectValidationError("Expired signature") + except InvalidSignatureError: + raise AtlassianConnectValidationError("JWT contained invalid signature") + except DecodeError: + raise AtlassianConnectValidationError("Could not decode JWT token") diff --git a/tests/sentry/integrations/jira/test_installed.py b/tests/sentry/integrations/jira/test_installed.py index a5326d93b12686..b283fe49a547f5 100644 --- a/tests/sentry/integrations/jira/test_installed.py +++ b/tests/sentry/integrations/jira/test_installed.py @@ -19,7 +19,6 @@ ) from sentry.testutils.asserts import ( assert_count_of_metric, - assert_failure_metric, assert_halt_metric, ) from sentry.testutils.cases import APITestCase @@ -88,19 +87,21 @@ def test_missing_body(self, mock_record_failure: MagicMock) -> None: ProjectManagementFailuresReason.INSTALLATION_STATE_MISSING ) + @responses.activate def test_missing_token(self) -> None: - self.get_error_response(**self.body(), status_code=status.HTTP_409_CONFLICT) + self.get_error_response(**self.body(), status_code=status.HTTP_400_BAD_REQUEST) + @responses.activate def test_invalid_token(self) -> None: self.get_error_response( **self.body(), extra_headers=dict(HTTP_AUTHORIZATION="invalid"), - status_code=status.HTTP_409_CONFLICT, + status_code=status.HTTP_400_BAD_REQUEST, ) @patch( - "sentry.integrations.jira.webhooks.installed.authenticate_asymmetric_jwt", - side_effect=AtlassianConnectValidationError(), + "sentry.integrations.utils.atlassian_connect.authenticate_asymmetric_jwt", + side_effect=AtlassianConnectValidationError("Failed to fetch key id"), ) @responses.activate def test_no_claims(self, mock_authenticate_asymmetric_jwt: MagicMock) -> None: @@ -109,11 +110,11 @@ def test_no_claims(self, mock_authenticate_asymmetric_jwt: MagicMock) -> None: self.get_error_response( **self.body(), extra_headers=dict(HTTP_AUTHORIZATION="JWT " + self.jwt_token_cdn()), - status_code=status.HTTP_409_CONFLICT, + status_code=status.HTTP_400_BAD_REQUEST, ) @patch( - "sentry.integrations.jira.webhooks.installed.authenticate_asymmetric_jwt", + "sentry.integrations.utils.atlassian_connect.authenticate_asymmetric_jwt", side_effect=ExpiredSignatureError(), ) @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") @@ -131,15 +132,15 @@ def test_expired_signature( # SLO metric asserts # ENSURE_CONTROL_SILO (success) -> VERIFY_INSTALLATION (failure) -> GET_CONTROL_RESPONSE (success) assert_count_of_metric(mock_record_event, EventLifecycleOutcome.STARTED, 3) - assert_count_of_metric(mock_record_event, EventLifecycleOutcome.FAILURE, 1) + assert_count_of_metric(mock_record_event, EventLifecycleOutcome.HALTED, 1) assert_count_of_metric(mock_record_event, EventLifecycleOutcome.SUCCESS, 2) - assert_failure_metric( + assert_halt_metric( mock_record_event, - ExpiredSignatureError(), + "Expired signature", ) @patch( - "sentry.integrations.jira.webhooks.installed.authenticate_asymmetric_jwt", + "sentry.integrations.utils.atlassian_connect.authenticate_asymmetric_jwt", side_effect=InvalidSignatureError(), ) @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") @@ -165,7 +166,7 @@ def test_invalid_signature( ) @patch( - "sentry.integrations.jira.webhooks.installed.authenticate_asymmetric_jwt", + "sentry.integrations.utils.atlassian_connect.authenticate_asymmetric_jwt", side_effect=DecodeError(), ) @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") From a104d076618c63a63c57191a8abbe08399b3b07e Mon Sep 17 00:00:00 2001 From: Gabe Villalobos Date: Mon, 13 Apr 2026 11:53:08 -0700 Subject: [PATCH 2/5] Minor refactors, adds JWT validation to BitBucket --- .../integrations/bitbucket/installed.py | 24 ++- .../integrations/jira/webhooks/installed.py | 2 +- .../integrations/utils/atlassian_connect.py | 5 +- .../integrations/bitbucket/test_installed.py | 196 +++++++++++++++++- 4 files changed, 217 insertions(+), 10 deletions(-) diff --git a/src/sentry/integrations/bitbucket/installed.py b/src/sentry/integrations/bitbucket/installed.py index 4be5cda97459d8..44db65b398a404 100644 --- a/src/sentry/integrations/bitbucket/installed.py +++ b/src/sentry/integrations/bitbucket/installed.py @@ -1,14 +1,24 @@ from django.http.request import HttpRequest from django.http.response import HttpResponseBase from django.views.decorators.csrf import csrf_exempt +from rest_framework import status from rest_framework.request import Request from rest_framework.response import Response from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import Endpoint, control_silo_endpoint +from sentry.integrations.base import IntegrationDomain from sentry.integrations.pipeline import ensure_integration from sentry.integrations.types import IntegrationProviderSlug +from sentry.integrations.utils.atlassian_connect import ( + AtlassianConnectTokenValidator, + AtlassianConnectValidationError, +) +from sentry.integrations.utils.metrics import ( + IntegrationPipelineViewEvent, + IntegrationPipelineViewType, +) from .integration import BitbucketIntegrationProvider @@ -27,7 +37,19 @@ def dispatch(self, request: HttpRequest, *args, **kwargs) -> HttpResponseBase: return super().dispatch(request, *args, **kwargs) def post(self, request: Request, *args, **kwargs) -> Response: - state = request.data + with IntegrationPipelineViewEvent( + interaction_type=IntegrationPipelineViewType.VERIFY_INSTALLATION, + domain=IntegrationDomain.SOURCE_CODE_MANAGEMENT, + provider_key=IntegrationProviderSlug.BITBUCKET.value, + ).capture() as lifecycle: + state = request.data + + try: + AtlassianConnectTokenValidator(request, method="POST").get_token() + except AtlassianConnectValidationError as e: + lifecycle.record_halt(halt_reason=e.message) + return self.respond({"detail": e.message}, status=status.HTTP_400_BAD_REQUEST) + data = BitbucketIntegrationProvider().build_integration(state) ensure_integration(IntegrationProviderSlug.BITBUCKET.value, data) diff --git a/src/sentry/integrations/jira/webhooks/installed.py b/src/sentry/integrations/jira/webhooks/installed.py index 496afbf5665690..4e6e280c607ff4 100644 --- a/src/sentry/integrations/jira/webhooks/installed.py +++ b/src/sentry/integrations/jira/webhooks/installed.py @@ -53,7 +53,7 @@ def post(self, request: Request, *args, **kwargs) -> Response: ) try: - AtlassianConnectTokenValidator(request).get_token() + AtlassianConnectTokenValidator(request, method="POST").get_token() except AtlassianConnectValidationError as e: lifecycle.record_halt(halt_reason=e.message) return self.respond({"detail": e.message}, status=status.HTTP_400_BAD_REQUEST) diff --git a/src/sentry/integrations/utils/atlassian_connect.py b/src/sentry/integrations/utils/atlassian_connect.py index 30dff793481682..62146ffffdfa93 100644 --- a/src/sentry/integrations/utils/atlassian_connect.py +++ b/src/sentry/integrations/utils/atlassian_connect.py @@ -157,8 +157,9 @@ def parse_integration_from_request(request: HttpRequest, provider: str) -> Integ class AtlassianConnectTokenValidator: - def __init__(self, request: HttpRequest) -> None: + def __init__(self, request: HttpRequest, method: str) -> None: self.request = request + self.method = method def get_token(self) -> str: try: @@ -177,7 +178,7 @@ def _validate_token(self, token: str) -> None: raise AtlassianConnectValidationError("Missing key_id (kid)") try: decoded_claims = authenticate_asymmetric_jwt(token, key_id) - verify_claims(decoded_claims, self.request.path, self.request.GET, self.request.method) + verify_claims(decoded_claims, self.request.path, self.request.GET, self.method) except InvalidKeyError: raise AtlassianConnectValidationError("JWT contained invalid key_id (kid)") except ExpiredSignatureError: diff --git a/tests/sentry/integrations/bitbucket/test_installed.py b/tests/sentry/integrations/bitbucket/test_installed.py index b7d0b37f575076..e0bdcdd3371b5c 100644 --- a/tests/sentry/integrations/bitbucket/test_installed.py +++ b/tests/sentry/integrations/bitbucket/test_installed.py @@ -1,21 +1,33 @@ from __future__ import annotations +from collections.abc import Mapping from typing import Any from unittest import mock +from unittest.mock import MagicMock, patch +import jwt as pyjwt import responses +from jwt import DecodeError, ExpiredSignatureError, InvalidSignatureError from sentry.integrations.bitbucket.installed import BitbucketInstalledEndpoint from sentry.integrations.bitbucket.integration import BitbucketIntegrationProvider, scopes from sentry.integrations.models.integration import Integration +from sentry.integrations.types import EventLifecycleOutcome +from sentry.integrations.utils.atlassian_connect import ( + AtlassianConnectValidationError, + get_query_hash, +) from sentry.models.project import Project from sentry.models.repository import Repository from sentry.organizations.services.organization.serial import serialize_rpc_organization from sentry.plugins.base import plugins from sentry.plugins.bases.issue2 import IssueTrackingPlugin2 from sentry.silo.base import SiloMode +from sentry.testutils.asserts import assert_count_of_metric, assert_halt_metric from sentry.testutils.cases import APITestCase from sentry.testutils.silo import assume_test_silo_mode, control_silo_test +from sentry.utils.http import absolute_uri +from tests.sentry.utils.test_jwt import RS256_KEY, RS256_PUB_KEY class BitbucketPlugin(IssueTrackingPlugin2): @@ -26,6 +38,35 @@ class BitbucketPlugin(IssueTrackingPlugin2): @control_silo_test class BitbucketInstalledEndpointTest(APITestCase): + kid = "bitbucket-test-kid" + + def _jwt_token( + self, + signing_algorithm: str, + key: str, + headers: Mapping[str, Any] | None = None, + ) -> str: + return pyjwt.encode( + { + "iss": self.client_key, + "aud": absolute_uri(), + "qsh": get_query_hash(self.path, method="POST", query_params={}), + }, + key, + algorithm=signing_algorithm, + headers={**(headers or {}), "alg": signing_algorithm}, + ) + + def jwt_token_cdn(self) -> str: + return self._jwt_token("RS256", RS256_KEY, headers={"kid": self.kid}) + + def add_cdn_response(self) -> None: + responses.add( + responses.GET, + f"https://connect-install-keys.atlassian.com/{self.kid}", + body=RS256_PUB_KEY, + ) + def setUp(self) -> None: self.provider = "bitbucket" self.path = "/extensions/bitbucket/installed/" @@ -103,7 +144,8 @@ def test_default_permissions(self) -> None: assert BitbucketInstalledEndpoint.authentication_classes == () assert BitbucketInstalledEndpoint.permission_classes == () - def test_installed_with_public_key(self) -> None: + @mock.patch("sentry.integrations.bitbucket.installed.AtlassianConnectTokenValidator") + def test_installed_with_public_key(self, mock_validator: MagicMock) -> None: response = self.client.post(self.path, data=self.team_data_from_bitbucket) assert response.status_code == 200 integration = Integration.objects.get(provider=self.provider, external_id=self.client_key) @@ -111,7 +153,8 @@ def test_installed_with_public_key(self) -> None: del integration.metadata["webhook_secret"] assert integration.metadata == self.metadata - def test_installed_without_public_key(self) -> None: + @mock.patch("sentry.integrations.bitbucket.installed.AtlassianConnectTokenValidator") + def test_installed_without_public_key(self, mock_validator: MagicMock) -> None: integration, created = Integration.objects.get_or_create( provider=self.provider, external_id=self.client_key, @@ -129,7 +172,8 @@ def test_installed_without_public_key(self) -> None: del integration_after.metadata["webhook_secret"] assert integration.metadata == integration_after.metadata - def test_installed_without_username(self) -> None: + @mock.patch("sentry.integrations.bitbucket.installed.AtlassianConnectTokenValidator") + def test_installed_without_username(self, mock_validator: MagicMock) -> None: """Test a user (not team) installation where the user has hidden their username from public view""" # Remove username to simulate privacy mode @@ -142,8 +186,11 @@ def test_installed_without_username(self) -> None: del integration.metadata["webhook_secret"] assert integration.metadata == self.user_metadata + @mock.patch("sentry.integrations.bitbucket.installed.AtlassianConnectTokenValidator") @mock.patch("sentry.integrations.bitbucket.integration.generate_token", return_value="0" * 64) - def test_installed_with_secret(self, mock_generate_token: mock.MagicMock) -> None: + def test_installed_with_secret( + self, mock_generate_token: mock.MagicMock, mock_validator: MagicMock + ) -> None: response = self.client.post(self.path, data=self.team_data_from_bitbucket) assert mock_generate_token.called assert response.status_code == 200 @@ -151,8 +198,9 @@ def test_installed_with_secret(self, mock_generate_token: mock.MagicMock) -> Non assert integration.name == self.username assert integration.metadata["webhook_secret"] == "0" * 64 + @mock.patch("sentry.integrations.bitbucket.installed.AtlassianConnectTokenValidator") @responses.activate - def test_plugin_migration(self) -> None: + def test_plugin_migration(self, mock_validator: MagicMock) -> None: with assume_test_silo_mode(SiloMode.CELL): accessible_repo = Repository.objects.create( organization_id=self.organization.id, @@ -201,8 +249,9 @@ def test_plugin_migration(self) -> None: assert Repository.objects.get(id=inaccessible_repo.id).integration_id is None + @mock.patch("sentry.integrations.bitbucket.installed.AtlassianConnectTokenValidator") @responses.activate - def test_disable_plugin_when_fully_migrated(self) -> None: + def test_disable_plugin_when_fully_migrated(self, mock_validator: MagicMock) -> None: with assume_test_silo_mode(SiloMode.CELL): project = Project.objects.create(organization_id=self.organization.id) @@ -239,3 +288,138 @@ def test_disable_plugin_when_fully_migrated(self) -> None: ) assert "bitbucket" not in [p.slug for p in plugins.for_project(project)] + + @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") + def test_missing_token(self, mock_record_event: MagicMock) -> None: + response = self.client.post(self.path, data=self.team_data_from_bitbucket) + assert response.status_code == 400 + assert response.data["detail"] == "Failed to retrieve token from request headers" + assert_count_of_metric(mock_record_event, EventLifecycleOutcome.STARTED, 3) + assert_count_of_metric(mock_record_event, EventLifecycleOutcome.HALTED, 1) + assert_count_of_metric(mock_record_event, EventLifecycleOutcome.SUCCESS, 2) + assert_halt_metric(mock_record_event, "Failed to retrieve token from request headers") + + @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") + def test_invalid_token(self, mock_record_event: MagicMock) -> None: + response = self.client.post( + self.path, + data=self.team_data_from_bitbucket, + HTTP_AUTHORIZATION="invalid", + ) + assert response.status_code == 400 + assert response.data["detail"] == "Failed to retrieve token from request headers" + assert_count_of_metric(mock_record_event, EventLifecycleOutcome.STARTED, 3) + assert_count_of_metric(mock_record_event, EventLifecycleOutcome.HALTED, 1) + assert_count_of_metric(mock_record_event, EventLifecycleOutcome.SUCCESS, 2) + assert_halt_metric(mock_record_event, "Failed to retrieve token from request headers") + + @patch( + "sentry.integrations.utils.atlassian_connect.authenticate_asymmetric_jwt", + side_effect=AtlassianConnectValidationError("Failed to fetch key id"), + ) + @responses.activate + def test_no_claims(self, mock_authenticate_asymmetric_jwt: MagicMock) -> None: + self.add_cdn_response() + response = self.client.post( + self.path, + data=self.team_data_from_bitbucket, + HTTP_AUTHORIZATION="JWT " + self.jwt_token_cdn(), + ) + assert response.status_code == 400 + + @patch( + "sentry.integrations.utils.atlassian_connect.authenticate_asymmetric_jwt", + side_effect=ExpiredSignatureError(), + ) + @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") + @responses.activate + def test_expired_signature( + self, mock_record_event: MagicMock, mock_authenticate_asymmetric_jwt: MagicMock + ) -> None: + self.add_cdn_response() + response = self.client.post( + self.path, + data=self.team_data_from_bitbucket, + HTTP_AUTHORIZATION="JWT " + self.jwt_token_cdn(), + ) + assert response.status_code == 400 + assert_count_of_metric(mock_record_event, EventLifecycleOutcome.STARTED, 3) + assert_count_of_metric(mock_record_event, EventLifecycleOutcome.HALTED, 1) + assert_count_of_metric(mock_record_event, EventLifecycleOutcome.SUCCESS, 2) + assert_halt_metric(mock_record_event, "Expired signature") + + @patch( + "sentry.integrations.utils.atlassian_connect.authenticate_asymmetric_jwt", + side_effect=InvalidSignatureError(), + ) + @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") + @responses.activate + def test_invalid_signature( + self, mock_record_event: MagicMock, mock_authenticate_asymmetric_jwt: MagicMock + ) -> None: + self.add_cdn_response() + response = self.client.post( + self.path, + data=self.team_data_from_bitbucket, + HTTP_AUTHORIZATION="JWT " + self.jwt_token_cdn(), + ) + assert response.status_code == 400 + assert_count_of_metric(mock_record_event, EventLifecycleOutcome.STARTED, 3) + assert_count_of_metric(mock_record_event, EventLifecycleOutcome.HALTED, 1) + assert_count_of_metric(mock_record_event, EventLifecycleOutcome.SUCCESS, 2) + assert_halt_metric(mock_record_event, "JWT contained invalid signature") + + @patch( + "sentry.integrations.utils.atlassian_connect.authenticate_asymmetric_jwt", + side_effect=DecodeError(), + ) + @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") + @responses.activate + def test_decode_error( + self, mock_record_event: MagicMock, mock_authenticate_asymmetric_jwt: MagicMock + ) -> None: + self.add_cdn_response() + response = self.client.post( + self.path, + data=self.team_data_from_bitbucket, + HTTP_AUTHORIZATION="JWT " + self.jwt_token_cdn(), + ) + assert response.status_code == 400 + assert_count_of_metric(mock_record_event, EventLifecycleOutcome.STARTED, 3) + assert_count_of_metric(mock_record_event, EventLifecycleOutcome.HALTED, 1) + assert_count_of_metric(mock_record_event, EventLifecycleOutcome.SUCCESS, 2) + assert_halt_metric(mock_record_event, "Could not decode JWT token") + + @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") + def test_without_key_id(self, mock_record_event: MagicMock) -> None: + response = self.client.post( + self.path, + data=self.team_data_from_bitbucket, + HTTP_AUTHORIZATION="JWT " + self._jwt_token("RS256", RS256_KEY, headers={}), + ) + assert response.status_code == 400 + assert_count_of_metric(mock_record_event, EventLifecycleOutcome.STARTED, 3) + assert_count_of_metric(mock_record_event, EventLifecycleOutcome.HALTED, 1) + assert_count_of_metric(mock_record_event, EventLifecycleOutcome.SUCCESS, 2) + assert_halt_metric(mock_record_event, "Missing key_id (kid)") + + @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") + @responses.activate + def test_with_invalid_key_id(self, mock_record_event: MagicMock) -> None: + responses.add( + responses.GET, + "https://connect-install-keys.atlassian.com/fake-kid", + body=b"Not Found", + status=404, + ) + response = self.client.post( + self.path, + data=self.team_data_from_bitbucket, + HTTP_AUTHORIZATION="JWT " + + self._jwt_token("RS256", RS256_KEY, headers={"kid": "fake-kid"}), + ) + assert response.status_code == 400 + assert_count_of_metric(mock_record_event, EventLifecycleOutcome.STARTED, 3) + assert_count_of_metric(mock_record_event, EventLifecycleOutcome.HALTED, 1) + assert_count_of_metric(mock_record_event, EventLifecycleOutcome.SUCCESS, 2) + assert_halt_metric(mock_record_event, "JWT contained invalid key_id (kid)") From b6e52072d866d054ca3f92057bc357c2210e4ec8 Mon Sep 17 00:00:00 2001 From: Gabe Villalobos Date: Mon, 13 Apr 2026 11:58:28 -0700 Subject: [PATCH 3/5] Changes default response message to something more generic --- src/sentry/integrations/bitbucket/installed.py | 7 +++++-- src/sentry/integrations/jira/webhooks/installed.py | 7 +++++-- src/sentry/integrations/utils/atlassian_connect.py | 4 +--- tests/sentry/integrations/bitbucket/test_installed.py | 4 ++-- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/sentry/integrations/bitbucket/installed.py b/src/sentry/integrations/bitbucket/installed.py index 44db65b398a404..a08de43fa6c5f3 100644 --- a/src/sentry/integrations/bitbucket/installed.py +++ b/src/sentry/integrations/bitbucket/installed.py @@ -47,8 +47,11 @@ def post(self, request: Request, *args, **kwargs) -> Response: try: AtlassianConnectTokenValidator(request, method="POST").get_token() except AtlassianConnectValidationError as e: - lifecycle.record_halt(halt_reason=e.message) - return self.respond({"detail": e.message}, status=status.HTTP_400_BAD_REQUEST) + lifecycle.record_halt(halt_reason=str(e)) + return self.respond( + {"detail": "Request Token Validation Failed"}, + status=status.HTTP_400_BAD_REQUEST, + ) data = BitbucketIntegrationProvider().build_integration(state) ensure_integration(IntegrationProviderSlug.BITBUCKET.value, data) diff --git a/src/sentry/integrations/jira/webhooks/installed.py b/src/sentry/integrations/jira/webhooks/installed.py index 4e6e280c607ff4..8b4615497a3518 100644 --- a/src/sentry/integrations/jira/webhooks/installed.py +++ b/src/sentry/integrations/jira/webhooks/installed.py @@ -55,8 +55,11 @@ def post(self, request: Request, *args, **kwargs) -> Response: try: AtlassianConnectTokenValidator(request, method="POST").get_token() except AtlassianConnectValidationError as e: - lifecycle.record_halt(halt_reason=e.message) - return self.respond({"detail": e.message}, status=status.HTTP_400_BAD_REQUEST) + lifecycle.record_halt(halt_reason=str(e)) + return self.respond( + {"detail": "Request Token Validation Failed"}, + status=status.HTTP_400_BAD_REQUEST, + ) data = JiraIntegrationProvider().build_integration(state) integration = ensure_integration(self.provider, data) diff --git a/src/sentry/integrations/utils/atlassian_connect.py b/src/sentry/integrations/utils/atlassian_connect.py index 62146ffffdfa93..aeb2f907f4b15b 100644 --- a/src/sentry/integrations/utils/atlassian_connect.py +++ b/src/sentry/integrations/utils/atlassian_connect.py @@ -16,9 +16,7 @@ class AtlassianConnectValidationError(Exception): - def __init__(self, message: str) -> None: - self.message = message - super().__init__(message) + pass def get_query_hash( diff --git a/tests/sentry/integrations/bitbucket/test_installed.py b/tests/sentry/integrations/bitbucket/test_installed.py index e0bdcdd3371b5c..9d0dc4d1daf768 100644 --- a/tests/sentry/integrations/bitbucket/test_installed.py +++ b/tests/sentry/integrations/bitbucket/test_installed.py @@ -293,7 +293,7 @@ def test_disable_plugin_when_fully_migrated(self, mock_validator: MagicMock) -> def test_missing_token(self, mock_record_event: MagicMock) -> None: response = self.client.post(self.path, data=self.team_data_from_bitbucket) assert response.status_code == 400 - assert response.data["detail"] == "Failed to retrieve token from request headers" + assert response.data["detail"] == "Request Token Validation Failed" assert_count_of_metric(mock_record_event, EventLifecycleOutcome.STARTED, 3) assert_count_of_metric(mock_record_event, EventLifecycleOutcome.HALTED, 1) assert_count_of_metric(mock_record_event, EventLifecycleOutcome.SUCCESS, 2) @@ -307,7 +307,7 @@ def test_invalid_token(self, mock_record_event: MagicMock) -> None: HTTP_AUTHORIZATION="invalid", ) assert response.status_code == 400 - assert response.data["detail"] == "Failed to retrieve token from request headers" + assert response.data["detail"] == "Request Token Validation Failed" assert_count_of_metric(mock_record_event, EventLifecycleOutcome.STARTED, 3) assert_count_of_metric(mock_record_event, EventLifecycleOutcome.HALTED, 1) assert_count_of_metric(mock_record_event, EventLifecycleOutcome.SUCCESS, 2) From 8d84ae833f830167aaf2fbe508583b5eb49792b6 Mon Sep 17 00:00:00 2001 From: Gabe Villalobos Date: Mon, 13 Apr 2026 14:20:40 -0700 Subject: [PATCH 4/5] Wraps pipeline building in context decorator --- src/sentry/integrations/bitbucket/installed.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/sentry/integrations/bitbucket/installed.py b/src/sentry/integrations/bitbucket/installed.py index a08de43fa6c5f3..175cf9919dd115 100644 --- a/src/sentry/integrations/bitbucket/installed.py +++ b/src/sentry/integrations/bitbucket/installed.py @@ -53,7 +53,7 @@ def post(self, request: Request, *args, **kwargs) -> Response: status=status.HTTP_400_BAD_REQUEST, ) - data = BitbucketIntegrationProvider().build_integration(state) - ensure_integration(IntegrationProviderSlug.BITBUCKET.value, data) + data = BitbucketIntegrationProvider().build_integration(state) + ensure_integration(IntegrationProviderSlug.BITBUCKET.value, data) - return self.respond() + return self.respond() From 1c486242f853a0dcc5d6e7851d252b0721b4c16c Mon Sep 17 00:00:00 2001 From: Gabe Villalobos Date: Wed, 15 Apr 2026 12:11:36 -0700 Subject: [PATCH 5/5] Moves exception messages to enums --- .../integrations/utils/atlassian_connect.py | 66 ++++++++++++++----- .../integrations/bitbucket/test_installed.py | 23 ++++--- .../integrations/jira/test_installed.py | 15 +++-- 3 files changed, 75 insertions(+), 29 deletions(-) diff --git a/src/sentry/integrations/utils/atlassian_connect.py b/src/sentry/integrations/utils/atlassian_connect.py index aeb2f907f4b15b..7c142ec7bf28e5 100644 --- a/src/sentry/integrations/utils/atlassian_connect.py +++ b/src/sentry/integrations/utils/atlassian_connect.py @@ -2,6 +2,7 @@ import hashlib from collections.abc import Mapping, Sequence +from enum import StrEnum import requests from django.http import HttpRequest @@ -15,6 +16,23 @@ from sentry.utils.http import absolute_uri, percent_encode +class AtlassianConnectFailureReason(StrEnum): + MISSING_AUTHORIZATION_HEADER = "Missing/Invalid authorization header" + NO_TOKEN_PARAMETER = "No token parameter" + NO_INTEGRATION_FOUND = "No integration found" + INVALID_SIGNATURE = "Signature is invalid" + EXPIRED_SIGNATURE = "Signature is expired" + QUERY_HASH_MISMATCH = "Query hash mismatch" + UNABLE_TO_VERIFY_ASYMMETRIC_JWT = "Unable to verify asymmetric installation JWT" + FAILED_TO_RETRIEVE_TOKEN = "Failed to retrieve token from request headers" + FAILED_TO_FETCH_KEY_ID = "Failed to fetch key_id (kid)" + MISSING_KEY_ID = "Missing key_id (kid)" + INVALID_KEY_ID = "JWT contained invalid key_id (kid)" + EXPIRED_SIGNATURE_TOKEN = "Expired signature" + INVALID_SIGNATURE_TOKEN = "JWT contained invalid signature" + COULD_NOT_DECODE_JWT = "Could not decode JWT token" + + class AtlassianConnectValidationError(Exception): pass @@ -49,7 +67,9 @@ def get_token(request: HttpRequest) -> str: auth_header: str = request.META["HTTP_AUTHORIZATION"] return auth_header.split(" ", 1)[1] except (KeyError, IndexError): - raise AtlassianConnectValidationError("Missing/Invalid authorization header") + raise AtlassianConnectValidationError( + AtlassianConnectFailureReason.MISSING_AUTHORIZATION_HEADER + ) def get_integration_from_jwt( @@ -63,7 +83,7 @@ def get_integration_from_jwt( # Extract the JWT token from the request's jwt query # parameter or the authorization header. if token is None: - raise AtlassianConnectValidationError("No token parameter") + raise AtlassianConnectValidationError(AtlassianConnectFailureReason.NO_TOKEN_PARAMETER) # Decode the JWT token, without verification. This gives # you a header JSON object, a claims JSON object, and a signature. claims = jwt.peek_claims(token) @@ -77,7 +97,7 @@ def get_integration_from_jwt( # by the add-on during the installation handshake integration = integration_service.get_integration(provider=provider, external_id=issuer) if not integration: - raise AtlassianConnectValidationError("No integration found") + raise AtlassianConnectValidationError(AtlassianConnectFailureReason.NO_INTEGRATION_FOUND) # Verify the signature with the sharedSecret and the algorithm specified in the header's # alg field. We only need the token + shared secret and do not want to provide an # audience to the JWT validation that is require to match. Bitbucket does give us an @@ -93,9 +113,13 @@ def get_integration_from_jwt( else jwt.decode(token, integration.metadata["shared_secret"], audience=False) ) except InvalidSignatureError as e: - raise AtlassianConnectValidationError("Signature is invalid") from e + raise AtlassianConnectValidationError( + AtlassianConnectFailureReason.INVALID_SIGNATURE + ) from e except ExpiredSignatureError as e: - raise AtlassianConnectValidationError("Signature is expired") from e + raise AtlassianConnectValidationError( + AtlassianConnectFailureReason.EXPIRED_SIGNATURE + ) from e verify_claims(decoded_claims, path, query_params, method) @@ -112,7 +136,7 @@ def verify_claims( # and comparing it against the qsh claim on the verified token. qsh = get_query_hash(path, method, query_params) if qsh != claims["qsh"]: - raise AtlassianConnectValidationError("Query hash mismatch") + raise AtlassianConnectValidationError(AtlassianConnectFailureReason.QUERY_HASH_MISMATCH) def authenticate_asymmetric_jwt(token: str | None, key_id: str) -> dict[str, str]: @@ -121,7 +145,7 @@ def authenticate_asymmetric_jwt(token: str | None, key_id: str) -> dict[str, str See: https://community.developer.atlassian.com/t/action-required-atlassian-connect-installation-lifecycle-security-improvements/49046 """ if token is None: - raise AtlassianConnectValidationError("No token parameter") + raise AtlassianConnectValidationError(AtlassianConnectFailureReason.NO_TOKEN_PARAMETER) headers = jwt.peek_header(token) key_response = requests.get(f"https://connect-install-keys.atlassian.com/{key_id}") public_key = key_response.content.decode("utf-8").strip() @@ -129,7 +153,9 @@ def authenticate_asymmetric_jwt(token: str | None, key_id: str) -> dict[str, str token, public_key, audience=absolute_uri(), algorithms=[headers.get("alg")] ) if not decoded_claims: - raise AtlassianConnectValidationError("Unable to verify asymmetric installation JWT") + raise AtlassianConnectValidationError( + AtlassianConnectFailureReason.UNABLE_TO_VERIFY_ASYMMETRIC_JWT + ) return decoded_claims @@ -163,7 +189,9 @@ def get_token(self) -> str: try: token = get_token(self.request) except Exception: - raise AtlassianConnectValidationError("Failed to retrieve token from request headers") + raise AtlassianConnectValidationError( + AtlassianConnectFailureReason.FAILED_TO_RETRIEVE_TOKEN + ) self._validate_token(token) return token @@ -171,17 +199,25 @@ def _validate_token(self, token: str) -> None: try: key_id = jwt.peek_header(token).get("kid") except DecodeError: - raise AtlassianConnectValidationError("Failed to fetch key_id (kid)") + raise AtlassianConnectValidationError( + AtlassianConnectFailureReason.FAILED_TO_FETCH_KEY_ID + ) if not key_id: - raise AtlassianConnectValidationError("Missing key_id (kid)") + raise AtlassianConnectValidationError(AtlassianConnectFailureReason.MISSING_KEY_ID) try: decoded_claims = authenticate_asymmetric_jwt(token, key_id) verify_claims(decoded_claims, self.request.path, self.request.GET, self.method) except InvalidKeyError: - raise AtlassianConnectValidationError("JWT contained invalid key_id (kid)") + raise AtlassianConnectValidationError(AtlassianConnectFailureReason.INVALID_KEY_ID) except ExpiredSignatureError: - raise AtlassianConnectValidationError("Expired signature") + raise AtlassianConnectValidationError( + AtlassianConnectFailureReason.EXPIRED_SIGNATURE_TOKEN + ) except InvalidSignatureError: - raise AtlassianConnectValidationError("JWT contained invalid signature") + raise AtlassianConnectValidationError( + AtlassianConnectFailureReason.INVALID_SIGNATURE_TOKEN + ) except DecodeError: - raise AtlassianConnectValidationError("Could not decode JWT token") + raise AtlassianConnectValidationError( + AtlassianConnectFailureReason.COULD_NOT_DECODE_JWT + ) diff --git a/tests/sentry/integrations/bitbucket/test_installed.py b/tests/sentry/integrations/bitbucket/test_installed.py index 9d0dc4d1daf768..969b5823a3840c 100644 --- a/tests/sentry/integrations/bitbucket/test_installed.py +++ b/tests/sentry/integrations/bitbucket/test_installed.py @@ -14,6 +14,7 @@ from sentry.integrations.models.integration import Integration from sentry.integrations.types import EventLifecycleOutcome from sentry.integrations.utils.atlassian_connect import ( + AtlassianConnectFailureReason, AtlassianConnectValidationError, get_query_hash, ) @@ -297,7 +298,9 @@ def test_missing_token(self, mock_record_event: MagicMock) -> None: assert_count_of_metric(mock_record_event, EventLifecycleOutcome.STARTED, 3) assert_count_of_metric(mock_record_event, EventLifecycleOutcome.HALTED, 1) assert_count_of_metric(mock_record_event, EventLifecycleOutcome.SUCCESS, 2) - assert_halt_metric(mock_record_event, "Failed to retrieve token from request headers") + assert_halt_metric( + mock_record_event, AtlassianConnectFailureReason.FAILED_TO_RETRIEVE_TOKEN + ) @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") def test_invalid_token(self, mock_record_event: MagicMock) -> None: @@ -311,11 +314,15 @@ def test_invalid_token(self, mock_record_event: MagicMock) -> None: assert_count_of_metric(mock_record_event, EventLifecycleOutcome.STARTED, 3) assert_count_of_metric(mock_record_event, EventLifecycleOutcome.HALTED, 1) assert_count_of_metric(mock_record_event, EventLifecycleOutcome.SUCCESS, 2) - assert_halt_metric(mock_record_event, "Failed to retrieve token from request headers") + assert_halt_metric( + mock_record_event, AtlassianConnectFailureReason.FAILED_TO_RETRIEVE_TOKEN + ) @patch( "sentry.integrations.utils.atlassian_connect.authenticate_asymmetric_jwt", - side_effect=AtlassianConnectValidationError("Failed to fetch key id"), + side_effect=AtlassianConnectValidationError( + AtlassianConnectFailureReason.FAILED_TO_FETCH_KEY_ID + ), ) @responses.activate def test_no_claims(self, mock_authenticate_asymmetric_jwt: MagicMock) -> None: @@ -346,7 +353,7 @@ def test_expired_signature( assert_count_of_metric(mock_record_event, EventLifecycleOutcome.STARTED, 3) assert_count_of_metric(mock_record_event, EventLifecycleOutcome.HALTED, 1) assert_count_of_metric(mock_record_event, EventLifecycleOutcome.SUCCESS, 2) - assert_halt_metric(mock_record_event, "Expired signature") + assert_halt_metric(mock_record_event, AtlassianConnectFailureReason.EXPIRED_SIGNATURE_TOKEN) @patch( "sentry.integrations.utils.atlassian_connect.authenticate_asymmetric_jwt", @@ -367,7 +374,7 @@ def test_invalid_signature( assert_count_of_metric(mock_record_event, EventLifecycleOutcome.STARTED, 3) assert_count_of_metric(mock_record_event, EventLifecycleOutcome.HALTED, 1) assert_count_of_metric(mock_record_event, EventLifecycleOutcome.SUCCESS, 2) - assert_halt_metric(mock_record_event, "JWT contained invalid signature") + assert_halt_metric(mock_record_event, AtlassianConnectFailureReason.INVALID_SIGNATURE_TOKEN) @patch( "sentry.integrations.utils.atlassian_connect.authenticate_asymmetric_jwt", @@ -388,7 +395,7 @@ def test_decode_error( assert_count_of_metric(mock_record_event, EventLifecycleOutcome.STARTED, 3) assert_count_of_metric(mock_record_event, EventLifecycleOutcome.HALTED, 1) assert_count_of_metric(mock_record_event, EventLifecycleOutcome.SUCCESS, 2) - assert_halt_metric(mock_record_event, "Could not decode JWT token") + assert_halt_metric(mock_record_event, AtlassianConnectFailureReason.COULD_NOT_DECODE_JWT) @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") def test_without_key_id(self, mock_record_event: MagicMock) -> None: @@ -401,7 +408,7 @@ def test_without_key_id(self, mock_record_event: MagicMock) -> None: assert_count_of_metric(mock_record_event, EventLifecycleOutcome.STARTED, 3) assert_count_of_metric(mock_record_event, EventLifecycleOutcome.HALTED, 1) assert_count_of_metric(mock_record_event, EventLifecycleOutcome.SUCCESS, 2) - assert_halt_metric(mock_record_event, "Missing key_id (kid)") + assert_halt_metric(mock_record_event, AtlassianConnectFailureReason.MISSING_KEY_ID) @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") @responses.activate @@ -422,4 +429,4 @@ def test_with_invalid_key_id(self, mock_record_event: MagicMock) -> None: assert_count_of_metric(mock_record_event, EventLifecycleOutcome.STARTED, 3) assert_count_of_metric(mock_record_event, EventLifecycleOutcome.HALTED, 1) assert_count_of_metric(mock_record_event, EventLifecycleOutcome.SUCCESS, 2) - assert_halt_metric(mock_record_event, "JWT contained invalid key_id (kid)") + assert_halt_metric(mock_record_event, AtlassianConnectFailureReason.INVALID_KEY_ID) diff --git a/tests/sentry/integrations/jira/test_installed.py b/tests/sentry/integrations/jira/test_installed.py index b283fe49a547f5..c2349525df44cd 100644 --- a/tests/sentry/integrations/jira/test_installed.py +++ b/tests/sentry/integrations/jira/test_installed.py @@ -14,6 +14,7 @@ from sentry.integrations.project_management.metrics import ProjectManagementFailuresReason from sentry.integrations.types import EventLifecycleOutcome from sentry.integrations.utils.atlassian_connect import ( + AtlassianConnectFailureReason, AtlassianConnectValidationError, get_query_hash, ) @@ -101,7 +102,9 @@ def test_invalid_token(self) -> None: @patch( "sentry.integrations.utils.atlassian_connect.authenticate_asymmetric_jwt", - side_effect=AtlassianConnectValidationError("Failed to fetch key id"), + side_effect=AtlassianConnectValidationError( + AtlassianConnectFailureReason.FAILED_TO_FETCH_KEY_ID + ), ) @responses.activate def test_no_claims(self, mock_authenticate_asymmetric_jwt: MagicMock) -> None: @@ -136,7 +139,7 @@ def test_expired_signature( assert_count_of_metric(mock_record_event, EventLifecycleOutcome.SUCCESS, 2) assert_halt_metric( mock_record_event, - "Expired signature", + AtlassianConnectFailureReason.EXPIRED_SIGNATURE_TOKEN, ) @patch( @@ -162,7 +165,7 @@ def test_invalid_signature( assert_count_of_metric(mock_record_event, EventLifecycleOutcome.SUCCESS, 2) assert_halt_metric( mock_record_event, - "JWT contained invalid signature", + AtlassianConnectFailureReason.INVALID_SIGNATURE_TOKEN, ) @patch( @@ -188,7 +191,7 @@ def test_decode_error( assert_count_of_metric(mock_record_event, EventLifecycleOutcome.SUCCESS, 2) assert_halt_metric( mock_record_event, - "Could not decode JWT token", + AtlassianConnectFailureReason.COULD_NOT_DECODE_JWT, ) @patch("sentry_sdk.set_tag") @@ -221,7 +224,7 @@ def test_without_key_id(self, mock_record_event: MagicMock) -> None: assert_count_of_metric(mock_record_event, EventLifecycleOutcome.SUCCESS, 2) assert_halt_metric( mock_record_event, - "Missing key_id (kid)", + AtlassianConnectFailureReason.MISSING_KEY_ID, ) @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") @@ -250,5 +253,5 @@ def test_with_invalid_key_id(self, mock_record_event: MagicMock) -> None: assert_count_of_metric(mock_record_event, EventLifecycleOutcome.SUCCESS, 2) assert_halt_metric( mock_record_event, - "JWT contained invalid key_id (kid)", + AtlassianConnectFailureReason.INVALID_KEY_ID, )