ref(integrations): Redirect GitHub installs straight to the link page#116412
Conversation
evanpurkhiser
requested review from
iamrajjoshi and
leeandher
and removed request for
a team
github-actions
Bot
added
the
Scope: Backend
![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 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f336f2f. Configure here.
| "sentry-integration-installation-link", | ||
| kwargs={"integration_slug": provider_id}, | ||
| query={"installationId": installation_id}, | ||
| ) |
There was a problem hiding this comment.
installationId lost moving from path param to query param
High Severity
The backend now redirects to /extensions/github/link/?installationId=12345, passing installationId as a query parameter. However, the IntegrationOrganizationLink frontend component reads installationId from useParams() (i.e., route path parameters). The /extensions/:integrationSlug/link/ route has no :installationId segment, so useParams() returns undefined for it. This breaks GitHub direct installs: the installation verification fetch is skipped, the security callout shows a warning, and the install button uses the wrong flow (finishInstallation instead of startFlow with the installation_id).
Additional Locations (1)
Reviewed by Cursor Bugbot for commit f336f2f. Configure here.
There was a problem hiding this comment.
Following up with a change for this part
When a user installs the Sentry GitHub App directly from github.com, GitHub redirects to `/extensions/github/setup/?setup_action=install&...` and `PipelineAdvancerView` was bouncing them onward to the legacy `/extensions/external-install/<provider>/<id>/` URL, which existed only to render the React org-picker. Skip the intermediate hop and redirect straight to the picker's real URL, `/extensions/<slug>/link/?installationId=<id>`. The trampoline page's fallback URL (for the rare case where the OAuth popup has no opener) gets the same treatment. To make this `reverse()`-able rather than a hardcoded path string, the `/extensions/<slug>/link/` route is registered as `sentry-integration-installation-link` (it was previously served only by the catch-all). The now-unused `integration-installation` URL is removed, and the matching carve-out in the integration request classifier middleware goes with it. The companion frontend cleanup (deleting `GitHubInstallRedirect` and its route entry) lands in a follow-up PR.
evanpurkhiser
force-pushed
the
evanpurkhiser/ref-integrations-redirect-github-installs-straight-to-the-link-page
branch
from
f336f2f to
f4a30ae
Compare
leeandher
left a comment
leeandher
left a comment
There was a problem hiding this comment.
looks good to me, but maybe worth checking API access logs for the previous url pattern to validate only github was using it
| re_path( | ||
| r"^extensions/external-install/(?P<provider_id>\w+)/(?P<installation_id>\w+)/$", | ||
| r"^extensions/(?P<integration_slug>[^/]+)/link/$", | ||
| react_page_view, | ||
| name="integration-installation", | ||
| name="sentry-integration-installation-link", | ||
| ), |
There was a problem hiding this comment.
Should we have a redirect for the old link? i dont have numbers or anything but I thought this was used by some of our integrators in some places (maybe vercel?)
There was a problem hiding this comment.
Are you sure? afaict this is only used for github
There was a problem hiding this comment.
I'll see if I can find access logs for this
There was a problem hiding this comment.
I think it may be my mis-remembering I last touched it while building out the hybrid cloud parser thing -- i might be conflating it with the integrationOrganizationLink page
There was a problem hiding this comment.
I probably am looking at it now, since the .../link page renders the same contents -- I don't know that external-install is used anywhere else, i'll defer to you 🙏
There was a problem hiding this comment.
Yeah I think this was JUST for github and was poorly named
evanpurkhiser
deleted the
evanpurkhiser/ref-integrations-redirect-github-installs-straight-to-the-link-page
branch
`IntegrationOrganizationLink` was wired at two different URL shapes — `/extensions/external-install/:integrationSlug/:installationId` for GitHub installs initiated from github.com, and `/extensions/:integrationSlug/link/` for everything else — and used `useParams` to pull the GitHub `installationId` out of the path on one of those routes. This made the component aware of two URL shapes for the same product flow. The companion backend change in #116412 redirects GitHub-initiated installs straight to the `link/` route with `installationId` in the query string, so the external-install URL no longer needs to resolve at all. This drops the React route entry and updates the link view to read `installationId` from the query string. The link view now only handles one URL shape. Depends on #116412 landing in production first.
2 participants
When a user installs the Sentry GitHub App directly from github.com, GitHub redirects to
/extensions/github/setup/?setup_action=install&...andPipelineAdvancerViewwas bouncing them onward to the legacy/extensions/external-install/<provider>/<id>/URL, which existed only to render the React org-picker. This skips the intermediate hop and redirects straight to the picker's real URL,/extensions/<slug>/link/?installationId=<id>. The trampoline page's fallback URL (for the rare case where the OAuth popup has no opener) gets the same treatment.To make this
reverse()-able rather than a hardcoded path string, the/extensions/<slug>/link/route is registered assentry-integration-installation-link(it was previously served only by the catch-all). The now-unusedintegration-installationURL is removed, and the matching carve-out in the integration request classifier middleware goes with it.The companion frontend cleanup (deleting
GitHubInstallRedirectand its route entry) lands in a follow-up PR.