diff --git a/src/sentry/identity/oauth2.py b/src/sentry/identity/oauth2.py index 6c1f6f0815b97a..b87397afc39bff 100644 --- a/src/sentry/identity/oauth2.py +++ b/src/sentry/identity/oauth2.py @@ -33,6 +33,7 @@ from sentry.pipeline.views.base import PipelineView from sentry.shared_integrations.exceptions import ApiError, ApiInvalidRequestError, ApiUnauthorized from sentry.users.models.identity import Identity +from sentry.utils.hashlib import sha256_text from sentry.utils.http import absolute_uri from .base import Provider @@ -44,6 +45,14 @@ ERR_TOKEN_RETRIEVAL = "Failed to retrieve token from the upstream service." +def _summarize_token_value(name: str, value: str | None) -> dict[str, Any]: + summary: dict[str, Any] = {f"{name}_present": value is not None} + if value: + summary[f"{name}_length"] = len(value) + summary[f"{name}_sha256"] = sha256_text(value).hexdigest() + return summary + + def _redirect_url(pipeline: IdentityPipeline) -> str: associate_url = reverse( "sentry-extension-setup", @@ -365,12 +374,12 @@ def dispatch(self, request: HttpRequest, pipeline: IdentityPipeline) -> HttpResp ) return pipeline.error(f"{ERR_INVALID_STATE}\nError: {error}") - if state != pipeline.fetch_state("state"): + expected_state = pipeline.fetch_state("state") + if state != expected_state: extra = { "error": "invalid_state", - "state": state, - "pipeline_state": pipeline.fetch_state("state"), - "code": code, + **_summarize_token_value("state", state), + **_summarize_token_value("pipeline_state", expected_state), } lifecycle.record_failure( IntegrationPipelineErrorReason.TOKEN_EXCHANGE_MISMATCHED_STATE, extra=extra diff --git a/tests/sentry/identity/test_oauth2.py b/tests/sentry/identity/test_oauth2.py index 3df0c8881e6d90..1031a0ac7c6d73 100644 --- a/tests/sentry/identity/test_oauth2.py +++ b/tests/sentry/identity/test_oauth2.py @@ -13,9 +13,11 @@ from sentry.identity.pipeline import IdentityPipeline from sentry.identity.providers.dummy import DummyProvider from sentry.integrations.types import EventLifecycleOutcome +from sentry.integrations.utils.metrics import IntegrationPipelineErrorReason from sentry.shared_integrations.exceptions import ApiUnauthorized from sentry.testutils.asserts import assert_failure_metric, assert_slo_metric from sentry.testutils.silo import control_silo_test +from sentry.utils.hashlib import sha256_text MockResponse = namedtuple("MockResponse", ["headers", "content"]) @@ -27,6 +29,7 @@ def setUp(self) -> None: sentry.identity.register(DummyProvider) super().setUp() self.request = RequestFactory().get("/") + self.request.session = Client().session self.request.subdomain = None def tearDown(self) -> None: @@ -157,6 +160,38 @@ def test_api_error(self, mock_record: MagicMock) -> None: assert_failure_metric(mock_record, ApiUnauthorized('{"token": "a-fake-token"}')) + def test_state_mismatch_logs_sanitized_values(self, mock_record: MagicMock) -> None: + malicious_state = "16LmDTzTt'; waitfor delay '0:0:15' --" + request = RequestFactory().get("/", {"state": malicious_state, "code": "abc"}) + request.session = Client().session + + pipeline = IdentityPipeline(request=request, provider_key="dummy") + expected_state = "expected-state-token" + pipeline.bind_state("state", expected_state) + + lifecycle = MagicMock() + context_manager = MagicMock() + context_manager.__enter__.return_value = lifecycle + context_manager.__exit__.return_value = None + + mock_event = MagicMock() + mock_event.capture.return_value = context_manager + + with patch("sentry.identity.oauth2.record_event", return_value=mock_event): + self.view.dispatch(request, pipeline) + lifecycle.record_failure.assert_called_once_with( + IntegrationPipelineErrorReason.TOKEN_EXCHANGE_MISMATCHED_STATE, + extra={ + "error": "invalid_state", + "state_present": True, + "state_length": len(malicious_state), + "state_sha256": sha256_text(malicious_state).hexdigest(), + "pipeline_state_present": True, + "pipeline_state_length": len(expected_state), + "pipeline_state_sha256": sha256_text(expected_state).hexdigest(), + }, + ) + @control_silo_test class OAuth2LoginViewTest(TestCase):