From 2ee53ad1cc9a3f7f07bdcad2ba9bd4c6d3e8b1d1 Mon Sep 17 00:00:00 2001 From: Evan Purkhiser Date: Tue, 21 Nov 2017 13:46:49 -0800 Subject: [PATCH] feat(auth): Support migration of auth identity id value Useful when an auth provider needs to migrate a id from a legacy provider identifying id key to a new key. --- src/sentry/auth/helper.py | 21 ++++++++++-- src/sentry/auth/provider.py | 18 ++++++++++ src/sentry/auth/providers/dummy.py | 8 +++-- .../frontend/test_auth_organization_login.py | 34 +++++++++++++++++++ 4 files changed, 76 insertions(+), 5 deletions(-) diff --git a/src/sentry/auth/helper.py b/src/sentry/auth/helper.py index 02fa73ba0a9aea..bb797d7e04f0f1 100644 --- a/src/sentry/auth/helper.py +++ b/src/sentry/auth/helper.py @@ -13,6 +13,7 @@ from django.utils.translation import ugettext_lazy as _ from sentry.app import locks +from sentry.auth.provider import MigratingIdentityId from sentry.auth.exceptions import IdentityNotValid from sentry.models import ( AuditLogEntry, AuditLogEntryEvent, AuthIdentity, AuthProvider, Organization, OrganizationMember, @@ -646,10 +647,12 @@ def _finish_login_pipeline(self, identity): their account. """ auth_provider = self.auth_provider + user_id = identity['id'] + lock = locks.get( 'sso:auth:{}:{}'.format( auth_provider.id, - md5_text(identity['id']).hexdigest(), + md5_text(user_id).hexdigest(), ), duration=5, ) @@ -657,9 +660,23 @@ def _finish_login_pipeline(self, identity): try: auth_identity = AuthIdentity.objects.select_related('user').get( auth_provider=auth_provider, - ident=identity['id'], + ident=user_id, ) except AuthIdentity.DoesNotExist: + auth_identity = None + + # Handle migration of identity keys + if not auth_identity and isinstance(user_id, MigratingIdentityId): + try: + auth_identity = AuthIdentity.objects.select_related('user').get( + auth_provider=auth_provider, + ident=user_id.legacy_id, + ) + auth_identity.update(ident=user_id.id) + except AuthIdentity.DoesNotExist: + auth_identity = None + + if not auth_identity: return self._handle_unknown_identity(identity) # If the User attached to this AuthIdentity is not active, diff --git a/src/sentry/auth/provider.py b/src/sentry/auth/provider.py index ead2402fd81184..98c676cc3cc005 100644 --- a/src/sentry/auth/provider.py +++ b/src/sentry/auth/provider.py @@ -1,10 +1,24 @@ from __future__ import absolute_import, print_function import logging +from collections import namedtuple from .view import ConfigureView +class MigratingIdentityId(namedtuple('MigratingIdentityId', ['id', 'legacy_id'])): + """ + MigratingIdentityId may be used in the ``id`` field of an identity + dictionary to facilitate migrating user identites from one identifying id + to another. + """ + __slots__ = () + + def __unicode__(self): + # Default to id when coercing for query lookup + return self.id + + class Provider(object): """ A provider indicates how authenticate should happen for a given service, @@ -62,6 +76,10 @@ def build_identity(self, state): The ``email`` and ``id`` keys are required, ``name`` is optional. + The ``id`` may be passed in as a ``MigratingIdentityId`` should the + the id key be migrating from one value to another and have multiple + lookup values. + If the identity can not be constructed an ``IdentityNotValid`` error should be raised. """ diff --git a/src/sentry/auth/providers/dummy.py b/src/sentry/auth/providers/dummy.py index 722b2373f0865e..70e36c7fc7474c 100644 --- a/src/sentry/auth/providers/dummy.py +++ b/src/sentry/auth/providers/dummy.py @@ -3,12 +3,14 @@ from django.http import HttpResponse from sentry.auth import Provider, AuthView +from sentry.auth.provider import MigratingIdentityId class AskEmail(AuthView): def dispatch(self, request, helper): if 'email' in request.POST: - helper.bind_state('email', request.POST['email']) + helper.bind_state('email', request.POST.get('email')) + helper.bind_state('legacy_email', request.POST.get('legacy_email')) return helper.next_step() return HttpResponse(DummyProvider.TEMPLATE) @@ -23,9 +25,9 @@ def get_auth_pipeline(self): def build_identity(self, state): return { - 'name': 'Dummy', - 'id': state['email'], + 'id': MigratingIdentityId(id=state['email'], legacy_id=state.get('legacy_email')), 'email': state['email'], + 'name': 'Dummy', } def refresh_identity(self, auth_identity): diff --git a/tests/sentry/web/frontend/test_auth_organization_login.py b/tests/sentry/web/frontend/test_auth_organization_login.py index 1f6241216f5e31..2d7f5a457b1d29 100644 --- a/tests/sentry/web/frontend/test_auth_organization_login.py +++ b/tests/sentry/web/frontend/test_auth_organization_login.py @@ -652,3 +652,37 @@ def test_swapped_identities(self): member2 = OrganizationMember.objects.get(id=member2.id) assert not getattr(member2.flags, 'sso:linked') assert getattr(member2.flags, 'sso:invalid') + + def test_flow_as_unauthenticated_existing_user_legacy_identity_migration(self): + organization = self.create_organization(name='foo', owner=self.user) + user = self.create_user('bar@example.com') + auth_provider = AuthProvider.objects.create( + organization=organization, + provider='dummy', + ) + user_ident = AuthIdentity.objects.create( + auth_provider=auth_provider, + user=user, + ident='foo@example.com', + ) + + path = reverse('sentry-auth-organization', args=[organization.slug]) + + resp = self.client.post(path, {'init': True}) + + assert resp.status_code == 200 + assert self.provider.TEMPLATE in resp.content.decode('utf-8') + + path = reverse('sentry-auth-sso') + + resp = self.client.post(path, { + 'email': 'foo@new-domain.com', + 'legacy_email': 'foo@example.com' + }) + + # Ensure the ident was migrated from the legacy identity + updated_ident = AuthIdentity.objects.get(id=user_ident.id) + assert updated_ident.ident == 'foo@new-domain.com' + + assert resp.status_code == 302 + assert resp['Location'] == 'http://testserver' + reverse('sentry-login')