diff --git a/src/sentry/integrations/jira/webhooks/installed.py b/src/sentry/integrations/jira/webhooks/installed.py index 168381b3595aac..d43c478f913100 100644 --- a/src/sentry/integrations/jira/webhooks/installed.py +++ b/src/sentry/integrations/jira/webhooks/installed.py @@ -1,5 +1,6 @@ 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 @@ -20,9 +21,6 @@ ) from sentry.utils import jwt -# Atlassian sends scanner bots to "test" Atlassian apps and they often hit this endpoint with a bad kid causing errors -INVALID_KEY_IDS = ["fake-kid"] - @control_silo_endpoint class JiraSentryInstalledWebhook(JiraWebhookBase): @@ -46,7 +44,14 @@ def post(self, request: Request, *args, **kwargs) -> Response: lifecycle.record_failure(ProjectManagementFailuresReason.INSTALLATION_STATE_MISSING) return self.respond(status=status.HTTP_400_BAD_REQUEST) - key_id = jwt.peek_header(token).get("kid") + 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, @@ -56,14 +61,34 @@ def post(self, request: Request, *args, **kwargs) -> Response: } ) - if key_id: - if key_id in INVALID_KEY_IDS: - lifecycle.record_halt(halt_reason="JWT contained invalid key_id (kid)") - return self.respond( - {"detail": "Invalid key id"}, status=status.HTTP_400_BAD_REQUEST - ) + 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 + ) data = JiraIntegrationProvider().build_integration(state) integration = ensure_integration(self.provider, data) diff --git a/tests/sentry/integrations/jira/test_installed.py b/tests/sentry/integrations/jira/test_installed.py index 41117dafd46efc..a5326d93b12686 100644 --- a/tests/sentry/integrations/jira/test_installed.py +++ b/tests/sentry/integrations/jira/test_installed.py @@ -6,6 +6,7 @@ import jwt import responses +from jwt import DecodeError, ExpiredSignatureError, InvalidSignatureError from rest_framework import status from sentry.constants import ObjectStatus @@ -16,7 +17,11 @@ AtlassianConnectValidationError, get_query_hash, ) -from sentry.testutils.asserts import assert_count_of_metric, assert_halt_metric +from sentry.testutils.asserts import ( + assert_count_of_metric, + assert_failure_metric, + assert_halt_metric, +) from sentry.testutils.cases import APITestCase from sentry.testutils.silo import control_silo_test from sentry.utils.http import absolute_uri @@ -49,9 +54,6 @@ def _jwt_token( headers={**(headers or {}), "alg": jira_signing_algorithm}, ) - def jwt_token_secret(self): - return self._jwt_token("HS256", self.shared_secret) - def jwt_token_cdn(self): return self._jwt_token("RS256", RS256_KEY, headers={"kid": self.kid}) @@ -110,18 +112,83 @@ def test_no_claims(self, mock_authenticate_asymmetric_jwt: MagicMock) -> None: status_code=status.HTTP_409_CONFLICT, ) + @patch( + "sentry.integrations.jira.webhooks.installed.authenticate_asymmetric_jwt", + side_effect=ExpiredSignatureError(), + ) @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") - @patch("sentry_sdk.set_tag") - def test_with_shared_secret(self, mock_set_tag: MagicMock, mock_record_event) -> None: - self.get_success_response( + @responses.activate + def test_expired_signature( + self, mock_record_event: MagicMock, mock_authenticate_asymmetric_jwt: MagicMock + ) -> None: + self.add_response() + + self.get_error_response( **self.body(), - extra_headers=dict(HTTP_AUTHORIZATION="JWT " + self.jwt_token_secret()), + extra_headers=dict(HTTP_AUTHORIZATION="JWT " + self.jwt_token_cdn()), + status_code=status.HTTP_400_BAD_REQUEST, + ) + # 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.SUCCESS, 2) + assert_failure_metric( + mock_record_event, + ExpiredSignatureError(), ) - integration = Integration.objects.get(provider="jira", external_id=self.external_id) - mock_set_tag.assert_any_call("integration_id", integration.id) - assert integration.status == ObjectStatus.ACTIVE - mock_record_event.assert_called_with(EventLifecycleOutcome.SUCCESS, None, False, None) + @patch( + "sentry.integrations.jira.webhooks.installed.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_response() + + self.get_error_response( + **self.body(), + extra_headers=dict(HTTP_AUTHORIZATION="JWT " + self.jwt_token_cdn()), + status_code=status.HTTP_400_BAD_REQUEST, + ) + # SLO metric asserts + # ENSURE_CONTROL_SILO (success) -> VERIFY_INSTALLATION (halt) -> GET_CONTROL_RESPONSE (success) + 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.jira.webhooks.installed.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_response() + + self.get_error_response( + **self.body(), + extra_headers=dict(HTTP_AUTHORIZATION="JWT " + self.jwt_token_cdn()), + status_code=status.HTTP_400_BAD_REQUEST, + ) + # SLO metric asserts + # ENSURE_CONTROL_SILO (success) -> VERIFY_INSTALLATION (halt) -> GET_CONTROL_RESPONSE (success) + 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_sdk.set_tag") @responses.activate @@ -138,7 +205,34 @@ def test_with_key_id(self, mock_set_tag: MagicMock) -> None: assert integration.status == ObjectStatus.ACTIVE @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") + def test_without_key_id(self, mock_record_event: MagicMock) -> None: + self.get_error_response( + **self.body(), + extra_headers=dict( + HTTP_AUTHORIZATION="JWT " + self._jwt_token("RS256", RS256_KEY, headers={}) + ), + status_code=status.HTTP_400_BAD_REQUEST, + ) + # SLO metric asserts + # ENSURE_CONTROL_SILO (success) -> VERIFY_INSTALLATION (halt) -> GET_CONTROL_RESPONSE (success) + 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, + ) + self.get_error_response( **self.body(), extra_headers=dict(