Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions src/sentry/auth/providers/oauth2.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
if TYPE_CHECKING:
from sentry.auth.helper import AuthHelper

logger = logging.getLogger(__name__)

ERR_INVALID_STATE = "An error occurred while validating your request."


Expand Down Expand Up @@ -129,7 +131,16 @@ def dispatch(self, request: HttpRequest, pipeline: AuthHelper) -> HttpResponseBa
code = request.GET.get("code")

if error:
return pipeline.error(error)
logger.warning(
"oauth2.auth.callback.error_param",
extra={
"provider": (
pipeline.provider.key if getattr(pipeline, "provider", None) else None
),
"error": error,
},
)
return pipeline.error(ERR_INVALID_STATE)

if state != pipeline.fetch_state("state"):
return pipeline.error(ERR_INVALID_STATE)
Expand All @@ -143,7 +154,7 @@ def dispatch(self, request: HttpRequest, pipeline: AuthHelper) -> HttpResponseBa
return pipeline.error(data["error_description"])

if "error" in data:
logging.info("Error exchanging token: %s", data["error"])
logger.info("Error exchanging token: %s", data["error"])
return pipeline.error("Unable to retrieve your token")

# we can either expect the API to be implicit and say "im looking for
Expand Down
9 changes: 8 additions & 1 deletion src/sentry/identity/oauth2.py
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,14 @@ def dispatch(self, request: HttpRequest, pipeline: IdentityPipeline) -> HttpResp
IntegrationPipelineErrorReason.TOKEN_EXCHANGE_MISMATCHED_STATE,
extra={"error": error},
)
return pipeline.error(f"{ERR_INVALID_STATE}\nError: {error}")
logger.warning(
"oauth2.callback.error_param",
extra={
"provider": pipeline.provider.key if pipeline.provider else None,
"error": error,
},
)
return pipeline.error(ERR_INVALID_STATE)

if state != pipeline.fetch_state("state"):
extra = {
Expand Down
27 changes: 26 additions & 1 deletion tests/sentry/auth/providers/test_oauth2.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
from collections.abc import Mapping
from functools import cached_property
from typing import Any
from unittest.mock import MagicMock

import pytest
from django.test import RequestFactory

from sentry.auth.exceptions import IdentityNotValid
from sentry.auth.providers.oauth2 import OAuth2Provider
from sentry.auth.providers.oauth2 import ERR_INVALID_STATE, OAuth2Callback, OAuth2Provider
from sentry.models.authidentity import AuthIdentity
from sentry.models.authprovider import AuthProvider
from sentry.testutils.cases import TestCase
Expand Down Expand Up @@ -48,3 +50,26 @@ def test_refresh_identity_without_refresh_token(self) -> None:
provider = DummyOAuth2Provider()
with pytest.raises(IdentityNotValid):
provider.refresh_identity(auth_identity)


@control_silo_test
class OAuth2CallbackAuthViewTest(TestCase):
def setUp(self) -> None:
super().setUp()
self.request_factory = RequestFactory()
self.callback = OAuth2Callback()
self.pipeline = MagicMock()
provider = MagicMock()
provider.key = "dummy"
self.pipeline.provider = provider

def test_error_query_param_returns_generic_message(self) -> None:
request = self.request_factory.get(
"/", {"error": "10'XOR(1*if(now()=sysdate(),sleep(15),0))XOR'Z"}
)
self.pipeline.error.return_value = object()

response = self.callback.dispatch(request, self.pipeline)

assert response is self.pipeline.error.return_value
self.pipeline.error.assert_called_once_with(ERR_INVALID_STATE)
19 changes: 18 additions & 1 deletion tests/sentry/identity/test_oauth2.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from requests.exceptions import SSLError

import sentry.identity
from sentry.identity.oauth2 import OAuth2CallbackView, OAuth2LoginView
from sentry.identity.oauth2 import ERR_INVALID_STATE, OAuth2CallbackView, OAuth2LoginView
from sentry.identity.pipeline import IdentityPipeline
from sentry.identity.providers.dummy import DummyProvider
from sentry.integrations.types import EventLifecycleOutcome
Expand Down Expand Up @@ -157,6 +157,23 @@ def test_api_error(self, mock_record: MagicMock) -> None:

assert_failure_metric(mock_record, ApiUnauthorized('{"token": "a-fake-token"}'))

def test_error_query_param_returns_generic_message(self, mock_record: MagicMock) -> None:
request = RequestFactory().get(
"/", {"error": "10'XOR(1*if(now()=sysdate(),sleep(15),0))XOR'Z"}
)
request.subdomain = None

pipeline = MagicMock()
provider = MagicMock()
provider.key = "dummy"
pipeline.provider = provider
pipeline.error.return_value = object()

response = self.view.dispatch(request, pipeline)

assert response is pipeline.error.return_value
pipeline.error.assert_called_once_with(ERR_INVALID_STATE)


@control_silo_test
class OAuth2LoginViewTest(TestCase):
Expand Down
Loading