Skip to content

Commit

Permalink
fix(integrations) Fix missing identity link on re-install (#13450)
Browse files Browse the repository at this point in the history
In a previous change I was trying to be careful and avoid
overwriting `default_auth_id` when it wasn't required. I missed
a scenario where a user:

1. Builds an integration.
2. Disconnects the integration, but keeps their Identity record
    & Integration records.
3. Reconnects the integration.

In this flow the new OrganizationIntegration would lack an
`default_auth_id` attribute as the Identity was not new.

Refs SEN-703
Fixes SENTRY-8Y9
Fixes SENTRY-AJQ
  • Loading branch information
markstory authored May 29, 2019
1 parent f65db76 commit 3fa3c13
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 7 deletions.
8 changes: 2 additions & 6 deletions src/sentry/integrations/pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,17 +104,14 @@ def _finish_pipeline(self, data):
'date_verified': timezone.now(),
}

new_id = None
try:
identity_model, created = Identity.objects.get_or_create(
idp=idp,
user=self.request.user,
external_id=identity['external_id'],
defaults=identity_data,
)
if created:
new_id = identity_model.id
else:
if not created:
identity_model.update(**identity_data)
except IntegrityError:
# If the external_id is already used for a different user or
Expand Down Expand Up @@ -146,13 +143,12 @@ def _finish_pipeline(self, data):
)
identity_model = Identity.reattach(
idp, identity['external_id'], self.request.user, identity_data)
new_id = identity_model.id

default_auth_id = None
if self.provider.needs_default_identity:
if not (identity and identity_model):
raise NotImplementedError('Integration requires an identity')
default_auth_id = new_id
default_auth_id = identity_model.id

org_integration = self.integration.add_organization(
self.organization,
Expand Down
53 changes: 52 additions & 1 deletion tests/sentry/integrations/test_pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@

from mock import patch

from sentry.models import Identity, Integration, OrganizationIntegration
from sentry.models import (
IdentityProvider,
Identity,
Integration,
OrganizationIntegration
)
from sentry.testutils import IntegrationTestCase
from sentry.integrations.example import (
ExampleIntegrationProvider,
Expand Down Expand Up @@ -205,6 +210,52 @@ def test_default_identity_does_update(self, *args):
identity = Identity.objects.get(external_id='AccountId')
assert org_integration.default_auth_id == identity.id

def test_existing_identity_becomes_default_auth_on_new_orgintegration(self, *args):
# The reinstall flow will result in an existing identity provider, identity
# and integration records. Ensure that the new organizationintegration gets
# a default_auth_id set.
self.provider.needs_default_identity = True
integration = Integration.objects.create(
provider=self.provider.key,
external_id=self.external_id,
metadata={
'url': 'https://example.com',
},
)
identity_provider = IdentityProvider.objects.create(
external_id=self.external_id,
type='plugin'
)
identity = Identity.objects.create(
idp_id=identity_provider.id,
external_id='AccountId',
user_id=self.user.id
)
self.pipeline.state.data = {
'external_id': self.external_id,
'name': 'Name',
'metadata': {'url': 'https://example.com'},
'user_identity': {
'type': 'plugin',
'external_id': 'AccountId',
'scopes': [],
'data': {
'access_token': 'token12345',
'expires_in': '123456789',
'refresh_token': 'refresh12345',
'token_type': 'typetype',
},
}
}
resp = self.pipeline.finish_pipeline()
self.assertDialogSuccess(resp)

org_integration = OrganizationIntegration.objects.get(
organization_id=self.organization.id,
integration_id=integration.id,
)
assert org_integration.default_auth_id == identity.id

@patch('sentry.mediators.plugins.Migrator.call')
def test_disabled_plugin_when_fully_migrated(self, call, *args):
Repository.objects.create(
Expand Down

0 comments on commit 3fa3c13

Please sign in to comment.