Skip to content

Conversation

@cursor
Copy link
Contributor

@cursor cursor bot commented Nov 19, 2025

Hardens OAuth2LoginView to strictly validate the OAuth state parameter.

Previously, a reverted change allowed the pipeline to advance without proper state validation if code, error, or state were merely present in the request. This enabled malicious query parameters (e.g., in error) to be reflected in PipelineError messages.

This PR reintroduces robust state validation in OAuth2LoginView.dispatch. Any callback attempt must now present an exact match for the expected OAuth state. Missing or mismatched states will immediately raise ERR_INVALID_STATE and record a TOKEN_EXCHANGE_MISMATCHED_STATE failure, preventing the pipeline from advancing and reflecting arbitrary query parameters.

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.


Open in Cursor Open in Web

Co-authored-by: jenn.muengtaweepongsa <jenn.muengtaweepongsa@sentry.io>
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Nov 19, 2025
@codecov
Copy link

codecov bot commented Nov 19, 2025

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
29872 1 29871 244
View the top 1 failed test(s) by shortest run time
tests.sentry.identity.test_oauth2.OAuth2LoginViewTest::test_callback_with_matching_state_advances_pipeline
Stack Traces | 0.063s run time
#x1B[1m#x1B[.../sentry/identity/test_oauth2.py#x1B[0m:232: in test_callback_with_matching_state_advances_pipeline
    response = self.view.dispatch(request, pipeline)
#x1B[1m#x1B[31m.venv/lib/python3.13.../django/utils/decorators.py#x1B[0m:48: in _wrapper
    return bound_method(*args, **kwargs)
#x1B[1m#x1B[31m.venv/lib/python3.13.../views/decorators/csrf.py#x1B[0m:65: in _view_wrapper
    return view_func(request, *args, **kwargs)
#x1B[1m#x1B[.../sentry/identity/oauth2.py#x1B[0m:267: in dispatch
    return pipeline.error(ERR_INVALID_STATE)
#x1B[1m#x1B[.../sentry/pipeline/base.py#x1B[0m:201: in error
    return render_to_response(
#x1B[1m#x1B[.../sentry/web/helpers.py#x1B[0m:42: in render_to_response
    response = HttpResponse(render_to_string(template, context, request))
#x1B[1m#x1B[.../sentry/web/helpers.py#x1B[0m:29: in render_to_string
    rendered = loader.render_to_string(template, context=context, request=request)
#x1B[1m#x1B[31m.venv/lib/python3.13.../django/template/loader.py#x1B[0m:62: in render_to_string
    return template.render(context, request)
#x1B[1m#x1B[31m.venv/lib/python3.13.../template/backends/django.py#x1B[0m:107: in render
    return self.template.render(context)
#x1B[1m#x1B[31m.venv/lib/python3.13.../django/template/base.py#x1B[0m:171: in render
    return self._render(context)
#x1B[1m#x1B[31m.venv/lib/python3.13.../django/test/utils.py#x1B[0m:114: in instrumented_test_render
    return self.nodelist.render(context)
#x1B[1m#x1B[31m.venv/lib/python3.13.../django/template/base.py#x1B[0m:1016: in render
    return SafeString("".join([node.render_annotated(context) for node in self]))
#x1B[1m#x1B[31m.venv/lib/python3.13.../django/template/base.py#x1B[0m:977: in render_annotated
    return self.render(context)
#x1B[1m#x1B[31m.venv/lib/python3.13.../django/template/loader_tags.py#x1B[0m:159: in render
    return compiled_parent._render(context)
#x1B[1m#x1B[31m.venv/lib/python3.13.../django/test/utils.py#x1B[0m:114: in instrumented_test_render
    return self.nodelist.render(context)
#x1B[1m#x1B[31m.venv/lib/python3.13.../django/template/base.py#x1B[0m:1016: in render
    return SafeString("".join([node.render_annotated(context) for node in self]))
#x1B[1m#x1B[31m.venv/lib/python3.13.../django/template/base.py#x1B[0m:977: in render_annotated
    return self.render(context)
#x1B[1m#x1B[31m.venv/lib/python3.13.../django/template/loader_tags.py#x1B[0m:159: in render
    return compiled_parent._render(context)
#x1B[1m#x1B[31m.venv/lib/python3.13.../django/test/utils.py#x1B[0m:114: in instrumented_test_render
    return self.nodelist.render(context)
#x1B[1m#x1B[31m.venv/lib/python3.13.../django/template/base.py#x1B[0m:1016: in render
    return SafeString("".join([node.render_annotated(context) for node in self]))
#x1B[1m#x1B[31m.venv/lib/python3.13.../django/template/base.py#x1B[0m:977: in render_annotated
    return self.render(context)
#x1B[1m#x1B[31m.venv/lib/python3.13.../django/template/library.py#x1B[0m:321: in render
    output = self.func(*resolved_args, **resolved_kwargs)
#x1B[1m#x1B[.../sentry/templatetags/sentry_helpers.py#x1B[0m:197: in get_sentry_version
    latest = options.get("sentry:latest_version") or current
#x1B[1m#x1B[.../sentry/options/manager.py#x1B[0m:312: in get
    result = self.store.get(opt, silent=silent)
#x1B[1m#x1B[.../sentry/options/store.py#x1B[0m:115: in get
    result = self.get_store(key, silent=silent)
#x1B[1m#x1B[.../sentry/options/store.py#x1B[0m:215: in get_store
    value = self.model.objects.get(key=key.name).value
#x1B[1m#x1B[31m.venv/lib/python3.13.../db/models/manager.py#x1B[0m:87: in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
#x1B[1m#x1B[31m.venv/lib/python3.13.../db/models/query.py#x1B[0m:631: in get
    num = len(clone)
#x1B[1m#x1B[31m.venv/lib/python3.13.../db/models/query.py#x1B[0m:368: in __len__
    self._fetch_all()
#x1B[1m#x1B[31m.venv/lib/python3.13.../db/models/query.py#x1B[0m:1954: in _fetch_all
    self._result_cache = list(self._iterable_class(self))
#x1B[1m#x1B[31m.venv/lib/python3.13.../db/models/query.py#x1B[0m:93: in __iter__
    results = compiler.execute_sql(
#x1B[1m#x1B[31m.venv/lib/python3.13.../models/sql/compiler.py#x1B[0m:1621: in execute_sql
    cursor = self.connection.cursor()
#x1B[1m#x1B[31m.venv/lib/python3.13.../django/utils/asyncio.py#x1B[0m:26: in inner
    return func(*args, **kwargs)
#x1B[1m#x1B[31m.venv/lib/python3.13.../backends/base/base.py#x1B[0m:320: in cursor
    return self._cursor()
#x1B[1m#x1B[.../db/postgres/decorators.py#x1B[0m:38: in inner
    return func(self, *args, **kwargs)
#x1B[1m#x1B[.../db/postgres/base.py#x1B[0m:114: in _cursor
    return super()._cursor()
#x1B[1m#x1B[31m.venv/lib/python3.13.../backends/base/base.py#x1B[0m:296: in _cursor
    self.ensure_connection()
#x1B[1m#x1B[31mE   RuntimeError: Database access not allowed, use the "django_db" mark, or the "db" or "transactional_db" fixtures to enable it.#x1B[0m

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants