GitHub OAuth Security Enhancement#17
Conversation
…#67876) We're adding one more step in the GitHub integration installation pipeline, namely GitHub OAuth2 authorize. This is transparent from the UX perspective as the data exchange happens without user interaction. The pipeline will now fail in these cases: - If there is a mismatch between currently authenticated GitHub user (derived from OAuth2 authorize step) and the user who installed the GitHub app (https://github.com/apps/sentry-io) - If there is a mismatch between `state` parameter supplied by user and pipeline signature - If GitHub could not generate correct `access_token` from the `code` (wrong or attempt of re-use of `code`). In all those cases, this error is shown: 
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| pipeline.fetch_state("github_authenticated_user") | ||
| != integration.metadata["sender"]["login"] | ||
| ): | ||
| return error(request, self.active_organization) |
There was a problem hiding this comment.
Missing key check causes KeyError on metadata access
High Severity
Accessing integration.metadata["sender"]["login"] without checking if "sender" exists in metadata will raise a KeyError. The build_integration method only adds "sender" to metadata when state.get("sender") is truthy, but the pipeline state stores the GitHub user as "github_authenticated_user", not "sender". When finish_pipeline calls ensure_integration, it overwrites the Integration's metadata with the output of build_integration (which lacks "sender"), stripping the sender data originally set by the webhook. On any subsequent re-linking attempt, this line crashes. The existing installation.py endpoint already guards against this with if "sender" not in integration.metadata.
Additional Locations (1)
| "code": request.GET.get("code"), | ||
| "client_id": github_client_id, | ||
| "client_secret": github_client_secret, | ||
| } |
There was a problem hiding this comment.
OAuth token exchange omits required redirect_uri parameter
Medium Severity
The OAuth authorization redirect includes redirect_uri, but the token exchange data dict omits it. Per the OAuth 2.0 spec (RFC 6749 §4.1.3), if redirect_uri was included in the authorization request, it must also be included—with an identical value—in the token request. The codebase's own OAuth2CallbackView.get_token_params() in identity/oauth2.py follows this pattern correctly. The comments in the new code even say "similar to OAuth2CallbackView.get_token_params" but the implementation diverges by leaving out redirect_uri.
Additional Locations (1)
2 participants
Test 4
Summary by CodeRabbit
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.
Replicated from ai-code-review-evaluation/sentry-coderabbit#4
Note
Medium Risk
Touches the GitHub installation entry flow and adds new OAuth token exchange/user verification logic; mistakes could block installs or create edge-case regressions, though scope is limited to setup paths.
Overview
Adds a new
OAuthLoginViewstep to the GitHub integration pipeline that performs a GitHub OAuth round-trip, exchanges the code for an access token, fetches the authenticated user (/user), and stores the login in pipeline state before continuing the install.Hardens
GitHubInstallationby reusinginstallation_idfrom pipeline state and rejecting installs when the authenticated GitHub user doesn’t match the integration’s recordedsender.login, and centralizes failure rendering via a sharederror()helper (including customer-domaindocument_origin).Updates
PipelineAdvancerViewto special-case onlygithubfor direct-from-GitHub install redirects, and expands tests to cover the OAuth redirect/callback flow plus new error cases (invalid state/installation and user mismatch).Written by Cursor Bugbot for commit 843d7c7. Configure here.