getsentry / sentry

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

Identities for Integrations #5963

Closed dcramer closed 4 years ago

dcramer commented 7 years ago

Identities

Currently there are two (2) solutions for storing identities from things like OAuth:

With the overhaul of Integrations it's worth considering a few option:

a. We create a new abstraction for identities and convert all things into it. b. We build a third system, to replace social_auth c. We keep using social_auth and rewrite it to suit our needs

The major problem with using social_auth is we can't use the same backend for multiple installations. For example, GitHub Enterprise, the provider might be 'ghe', but it's actually a combination of ('ghe', domain). Atlassian has similar concerns with Jira, where it's ('jira', domain).

Another minor note, is we need identities to capture scopes, and support "upgrade" procedures when a use-case requires additional scope. Zeus implements an example of this.

Option A: Unified Abstraction

This would take our SSO code and use it as a baseline implementation for a new identity solution. It was modeled with pipelines in mind and has been implemented to support both OAuth and SAML style implementations, which covers every immediate use case we have.

The downside here is AuthIdentity is coupled to AuthProvider, and that's a problem. We need a singular Identity (provider class + external_id), and we should share. That could look like:

This is a bit tedious, but from a data model, it should work well. It's also similar to how Integrations as a whole are structured and future proofs us to use Identity for anything else.

Option B: A new system

I'm generally -1 on this as it creates yet another duplicated code path, and increases maintenance burden. It, however, does have the advantage that it won't conflict with any other system and is arguably the easiest to implement.

Option C: Use social_auth

Also -1 on this. We'd need to refactor social_auth to support things like GHE + Atlassian Connect. We'd also likely want to make changes to UserSocialAuth per Option A so tokens can be shared across multiple accounts. That would ensure we can also resolve the limit of a GitHub account only being able to associate with a single User account. We may not ACTUALLY want to share an identity between multiple accounts, but given the way the system works right now (and our inability to enforce a globally unique user), it's probably worth investing into.

dcramer commented 7 years ago

@macqueen @mattrobenolt @mitsuhiko let me know your thoughts on this. I obviously prefer Option A.

mitsuhiko commented 7 years ago

Same. A is what I would suggest but I am not sure how much complexity is there in extending it to replace social auth.

dcramer commented 7 years ago

@mitsuhiko a lot would go into getting the base model setup, writing new data into it, and migrating all old data to the rows. Once those are done, we could simply swap how the tokens are read in various systems.

Alternatively, we can just ignore legacy integrations, and focus on SSO + new Integrations system for Option A.

dcramer commented 7 years ago

I'm going to initially just add in the new models, and not link up AuthIdentity or UserSocialAuth.

So rough phases might be:

  1. Schema and refactor of sentry.auth helpers
  2. Tie Identity into AuthIdentity
  3. Migratation path for UserSocialAuth
dcramer commented 7 years ago

One thing I didn't draft up yet is how we deal with provider being identical, but the instance of it being different.

We could make a 'Provider' class which just contains instances of the provider implementation.

This would be like:

For things that are stock (github), we could automatically register them?

BYK commented 4 years ago

I think we did this to an extent already.