getsentry / sentry

Developer-first error tracking and performance monitoring
https://sentry.io
Other
37.24k stars 4k forks source link

Gaps in Integrations Platform external install flow #55794

Closed shaunpersad closed 1 week ago

shaunpersad commented 8 months ago

Problem Statement

We're trying to use the external install flow described here: https://docs.sentry.io/product/integrations/integration-platform/public-integration/

However, while the flow looks like a standard OAuth2 flow, it does not appear to support passing a state query param (and consequently receiving the param back on redirect): https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.1

Support for this param has the security benefits outlined in the spec, but most relevant is that it would allow us to explicitly control where external installs happen. In our case, we'd only want them to happen from within our dashboard.

Typically with other oauth flows, we pass in a state query param to the authorization URL that is actually a signed JWT that contains relevant context (account and other of our resource IDs that this install operating on). When the flow is redirected back to us, we can then validate that the flow started from us, and also extract the important context to continue with the next steps.

Without this param, we're not only unable to validate where the install started, but we'd be unable to continue since we'd be missing any context about what to do next, e.g. we wouldn't know which of our accounts wants to install our Sentry integration, and on what resources.

Solution Brainstorm

No response

Product Area

Settings - Integrations

getsantry[bot] commented 8 months ago

Assigning to @getsentry/support for routing ⏲️

getsantry[bot] commented 8 months ago

Routing to @getsentry/product-owners-settings-integrations for triage ⏲️

Dhrumil-Sentry commented 8 months ago

@shaunpersad did you folks consider installation webhooks to help solve this problem?

shaunpersad commented 8 months ago

I'm not entirely sure how the installation webhook can help solve the problems outlined above that the state param would solve:

Without this param, we're not only unable to validate where the install started, but we'd be unable to continue since we'd be missing any context about what to do next, e.g. we wouldn't know which of our accounts wants to install our Sentry integration, and on what resources.

From the looks of the installation webhook's payload, it doesn't appear to be able to allow any additional data that we could use to either validate the install was triggered from our dash, or to give us the additional context about the install.

To be clear, the state param from a traditional oauth2 flow allows us to include verifiable additional context that is specific to us (our account IDs, our resource IDs, etc.). It's not sufficient to simply get which Sentry user performed the install, because we still need to know which of our users and resources maps to the install.

DanielAbergel commented 1 month ago

Hey team :) I am part of the monday.com team responsible for building the new Sentry integration. We encountered the same issue that @shaunpersad mentioned, and we need this ability to move forward. thanks

Dhrumil-Sentry commented 1 month ago

@DanielAbergel thanks for this report, I've added this to our backlog and we'll update the issue as we have more details or questions

JoshFerge commented 1 week ago

hi @DanielAbergel @shaunpersad i've just added the ability to pass a state parameter to the /external-install/ endpoint which then gets passed to the redirect during setup. let us know if this solves.

JoshFerge commented 1 week ago

(should deploy in about 15 minutes)

DanielAbergel commented 1 week ago

Hey @JoshFerge I just verified it, and it's working! Thanks a lot for all the help, we really appreciate it!