Merge pull request #107 from pennyfx/bid_fix

Bug fix to ensure that if user changes email address on AMO that we can still find the account on our side

apps/person/models.py

@@ -69,7 +69,13 @@ class Profile(models.Model):
             if row:
                 for i in range(len(row)):
                     data[columns[i]] = row[i]
-
+                    
+        if 'email' in data and self.user.email != data['email']:
+            log.info('User (%s) updated email (%s)' % (
+                self.user.username, self.user.email))
+            self.user.email = data['email']
+            self.user.save()
+            
         if 'display_name' in data:
             if data['display_name']:
                 names = data['display_name'].split(' ')

apps/person/tests.py

@@ -5,8 +5,12 @@ person.urls
 from django.contrib.auth.models import User
 from django.core.urlresolvers import reverse
 from django.test import TestCase
+from mock import Mock
 from nose.tools import eq_
+from waffle.models import Switch
 
+from amo.authentication import  AMOAuthentication
+from django_browserid import auth
 from person.models import Profile
 
 
@@ -43,3 +47,46 @@ class ProfileTest(TestCase):
     def test_fake_profile(self):
         resp = self.client.get(reverse('person_public_profile', args=['xxx']))
         eq_(404, resp.status_code)
+
+
+class BrowserIDLoginTest(TestCase):
+
+    TESTEMAIL = 'jdoe@example.com'
+
+    def setUp(self):
+        Switch.objects.create(
+                name='browserid-login',
+                active=True)
+        self.user = User.objects.create(
+                username='123', email=self.TESTEMAIL)
+        Profile.objects.create(user=self.user, nickname='jdoe')
+
+        # Mocking BrowserIDBackend
+        class BIDBackend():
+            def verify(self, assertion, site):
+                return {'email': assertion}
+        self.BIDBackend = auth.BrowserIDBackend
+        auth.BrowserIDBackend = BIDBackend
+
+    def tearDown(self):
+        auth.BrowserIDBackend = self.BIDBackend
+
+    def test_existing_user_login(self):
+        AMOAuthentication.auth_browserid_authenticate = Mock(
+                return_value={'id': '123'})
+        response = self.client.post(reverse('browserid_login'),
+                        {'assertion': self.TESTEMAIL})
+        eq_(response.status_code, 200)
+        assert self.user.is_authenticated()
+
+    def test_user_changed_email_on_AMO(self):
+        auth.BrowserIDBackend.verify = Mock(return_value={'email': 'some@example.com'})
+        AMOAuthentication.auth_browserid_authenticate = Mock(
+                return_value={'id': '123', 'email': 'some@example.com'})
+        response = self.client.post(reverse('browserid_login'),
+                        {'assertion': 'some-assertion'})
+        eq_(response.status_code, 200)
+        assert self.user.is_authenticated()        
+        assert User.objects.filter(email='some@example.com')
+        self.assertRaises(User.DoesNotExist,
+                User.objects.get, email=self.TESTEMAIL)

apps/person/views.py

@@ -10,7 +10,7 @@ from django.http import Http404
 from django import http
 
 from amo.authentication import  AMOAuthentication
-from django_browserid.auth import BrowserIDBackend
+from django_browserid import auth as browserid_auth
 
 import waffle
 
@@ -112,7 +112,7 @@ def browserid_authenticate(request, assertion):
     Verify a BrowserID login attempt. If the BrowserID assertion is
     good, but no account exists on flightdeck, create one.
     """
-    backend = BrowserIDBackend()
+    backend = browserid_auth.BrowserIDBackend()
     
     result = backend.verify(assertion, settings.SITE_URL)
     if not result:
@@ -121,24 +121,23 @@ def browserid_authenticate(request, assertion):
     email = result['email']
     
     amouser = AMOAuthentication.auth_browserid_authenticate(email)
+    
     if amouser == None:
         return (None,None)
     
-    users = User.objects.filter(email=email)
+    users = User.objects.filter(username=amouser['id'])
     if len(users) == 1:
         try:
             profile = users[0].get_profile()
         except Profile.DoesNotExist:
             profile = Profile(user=users[0])
-        profile.user.backend = 'django_browserid.auth.BrowserIDBackend'
-        return (profile, None)
-        
-    username = amouser['id']
-    user = User.objects.create(username=username, email=email)
+            profile.user.save()
+    else:
+        user = User.objects.create(username=amouser['id'], email=email)
+        profile = Profile(user=user)
+        profile.user.save()
     
-    profile = Profile(user=user)
     profile.user.backend = 'django_browserid.auth.BrowserIDBackend'
-    profile.user.save()    
     profile.update_from_AMO(amouser)
     
     return (profile, None)