diff --git a/src/sentry/integrations/pipeline.py b/src/sentry/integrations/pipeline.py index abca8355541f73..825538918ebe28 100644 --- a/src/sentry/integrations/pipeline.py +++ b/src/sentry/integrations/pipeline.py @@ -104,7 +104,6 @@ def _finish_pipeline(self, data): 'date_verified': timezone.now(), } - new_id = None try: identity_model, created = Identity.objects.get_or_create( idp=idp, @@ -112,9 +111,7 @@ def _finish_pipeline(self, data): 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 @@ -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, diff --git a/tests/sentry/integrations/test_pipeline.py b/tests/sentry/integrations/test_pipeline.py index e7e3a5ac039604..83d6006bc9f3ff 100644 --- a/tests/sentry/integrations/test_pipeline.py +++ b/tests/sentry/integrations/test_pipeline.py @@ -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, @@ -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(