LongTermClipper preserving LDAP data

README.rst

@@ -175,6 +175,10 @@ usernames won't be reused later.
 This adapter also handles getting basic information about the user from SPI's
 LDAP.
 
+**Important:** If you are building on top of *allauth*, take care to preserve
+the ``extra_data['ldap']`` of ``SocialAccount`` instances related to *Clipper*
+(where ``provider_id`` is ``clipper`` or ``clipper_inactive``).
+
 Configuration
   Set ``SOCIALACCOUNT_ADAPTER='allauth_ens.adapter.LongTermClipperAccountAdapter'``
   in `settings.py`
@@ -199,7 +203,6 @@ Customize
   You can customize the SocialAccountAdapter by inheriting
   ``allauth_ens.adapter.LongTermClipperAccountAdapter``. You might want to
   modify ``get_username(clipper, data)`` to change the default username format.
-  This function is used to disambiguate in the account deprecation process.
   By default, ``get_username`` raises a ``ValueError`` when the connexion to the
   LDAP failed or did not allow to retrieve the user's entrance year. Overriding
   ``get_username`` (as done in the example website) allows to get rid of that

allauth_ens/adapter.py

@@ -25,8 +25,8 @@ class LongTermClipperAccountAdapter(DefaultSocialAccountAdapter):
         If a clipper connection has already existed with the uid, it checks
         that this connection still belongs to the user it was associated with.
 
-        This check is performed by comparing the generated username corresponding 
-        to this connection with the old one.
+        This check is performed by comparing the entrance years provided by the
+        LDAP.
 
         If the check succeeds, it simply reactivates the clipper connection as
         belonging to the associated user.
@@ -52,8 +52,13 @@ class LongTermClipperAccountAdapter(DefaultSocialAccountAdapter):
         ldap_data = get_ldap_infos(clipper_uid)
         sociallogin._ldap_data = ldap_data
 
-        if old_conn.user.username != self.get_username(clipper_uid, ldap_data):
-            # The admission year is different
+        if ldap_data is None or 'entrance_year' not in ldap_data:
+            raise ValueError("No entrance year in LDAP data")
+
+        old_conn_entrance_year = (
+            old_conn.extra_data.get('ldap', {}).get('entrance_year'))
+
+        if old_conn_entrance_year != ldap_data['entrance_year']:
             # We cannot reuse this SocialAccount, so we need to invalidate
             # the email address of the previous user to prevent conflicts
             # if a new SocialAccount is created
@@ -72,8 +77,7 @@ class LongTermClipperAccountAdapter(DefaultSocialAccountAdapter):
 
     def get_username(self, clipper_uid, data):
         """
-        Util function to generate a unique username, by default 'clipper@promo'
-        This is used to disambiguate and recognize if the person is the same
+        Util function to generate a unique username, by default 'clipper@promo'.
         """
         if data is None or 'entrance_year' not in data:
             raise ValueError("No entrance year in LDAP data")
@@ -114,7 +118,7 @@ class LongTermClipperAccountAdapter(DefaultSocialAccountAdapter):
         get_account_adapter().populate_username(request, user)
 
         # Save extra data (only once)
-        sociallogin.account.extra_data = sociallogin.extra_data = ldap_data
+        sociallogin.account.extra_data['ldap'] = ldap_data
         sociallogin.save(request)
         sociallogin.account.save()
 
@@ -175,15 +179,22 @@ def install_longterm_adapter(fake=False):
         else:
             user.save()
             cases.append(user.username)
-        if SocialAccount.objects.filter(provider='clipper',
-                                        uid=clipper_uid).exists():
-            logs["updated"].append((clipper_uid, user.username))
-            continue
-        sa = SocialAccount(user=user, provider='clipper',
-                           uid=clipper_uid, extra_data=data)
-        if not fake:
-            sa.save()
-        logs["created"].append((clipper_uid, user.username))
+
+        try:
+            sa = SocialAccount.objects.get(provider='clipper', uid=clipper_uid)
+            if not sa.extra_data.get('ldap'):
+                sa.extra_data['ldap'] = data
+                if not fake:
+                    sa.save(update_fields=['extra_data'])
+                logs["updated"].append((clipper_uid, user.username))
+        except SocialAccount.DoesNotExist:
+            sa = SocialAccount(
+                provider='clipper', uid=clipper_uid,
+                user=user, extra_data={'ldap': data},
+            )
+            if not fake:
+                sa.save()
+            logs["created"].append((clipper_uid, user.username))
 
     logs["unmodified"] = User.objects.exclude(username__in=cases)\
                                      .values_list("username", flat=True)

allauth_ens/providers/clipper/provider.py

@@ -37,8 +37,23 @@ class ClipperProvider(CASProvider):
         ]
 
     def extract_extra_data(self, data):
+        """
+        If LongTermClipperAccountAdapter is in use, keep the data retrieved
+        from the LDAP server.
+        """
+        from allauth.socialaccount.models import SocialAccount  # noqa
         extra_data = super(ClipperProvider, self).extract_extra_data(data)
         extra_data['email'] = self.extract_email(data)
+
+        # Preserve LDAP data at all cost.
+        try:
+            clipper_account = SocialAccount.objects.get(
+                provider=self.id, uid=self.extract_uid(data))
+            if 'ldap' in clipper_account.extra_data:
+                extra_data['ldap'] = clipper_account.extra_data['ldap']
+        except SocialAccount.DoesNotExist:
+            pass
+
         return extra_data
 
     def message_suggest_caslogout_on_logout(self, request):

allauth_ens/providers/clipper/tests.py

@@ -17,6 +17,18 @@ class ClipperProviderTests(CASTestCase):
         u = User.objects.get(username='clipper_uid')
         self.assertEqual(u.email, 'clipper_uid@clipper.ens.fr')
 
+    def test_extra_data_keeps_ldap_data(self):
+        clipper_conn = self.u.socialaccount_set.create(
+            uid='user', provider='clipper',
+            extra_data={'ldap': {'aa': 'bb'}},
+        )
+
+        self.client_cas_login(
+            self.client, provider_id='clipper', username='user')
+
+        clipper_conn.refresh_from_db()
+        self.assertEqual(clipper_conn.extra_data['ldap'], {'aa': 'bb'})
+
 
 class ClipperViewsTests(CASViewTestCase):
 

allauth_ens/tests.py

@@ -1,3 +1,5 @@
+from __future__ import unicode_literals
+
 import re
 
 import django
@@ -8,6 +10,7 @@ from django.test import TestCase, override_settings
 
 from allauth.socialaccount.models import SocialAccount
 
+import six
 from allauth_cas.test.testcases import CASTestCase
 from fakeldap import MockLDAP
 from mock import patch
@@ -161,8 +164,12 @@ class LongTermClipperTests(CASTestCase):
         _mock_ldap.reset()
 
     def _setup_ldap(self, promo=12, username="test"):
+        try:
+            buid = six.binary_type(username, 'utf-8')
+        except TypeError:
+            buid = six.binary_type(username)
         _mock_ldap.directory['dc=spi,dc=ens,dc=fr'] = {
-            'uid': [username],
+            'uid': [buid],
             'cn': [b'John Smith'],
             'mailRoutingAddress': [b'test@clipper.ens.fr'],
             'homeDirectory': [b'/users/%d/phy/test/' % promo],
@@ -188,6 +195,7 @@ class LongTermClipperTests(CASTestCase):
 
         sa = list(SocialAccount.objects.all())[-1]
         self.assertEqual(sa.user.id, u.id)
+        self.assertEqual(sa.extra_data['ldap']['entrance_year'], '12')
 
     def test_connect_disconnect(self):
         self._setup_ldap()
@@ -312,9 +320,11 @@ class LongTermClipperTests(CASTestCase):
                                   username='test')
         user1 = r.context["user"]
         nsa1 = SocialAccount.objects.count()
+        conn = user1.socialaccount_set.get(provider='clipper')
         self.assertEqual(user1.id, user0.id)
         self.assertEqual(nsa1, nsa0)
         self.assertEqual(user1.username, "test@12")
+        self.assertEqual(conn.extra_data['ldap']['entrance_year'], '12')
 
     def test_longterm_installer_from_djangocas(self):
         with self.settings(SOCIALACCOUNT_ADAPTER=
@@ -332,9 +342,11 @@ class LongTermClipperTests(CASTestCase):
                                   username='test')
         user1 = r.context["user"]
         nsa1 = SocialAccount.objects.count()
+        conn = user1.socialaccount_set.get(provider='clipper')
         self.assertEqual(user1.id, user0.id)
         self.assertEqual(nsa1, nsa0 + 1)
         self.assertEqual(user1.username, "test@12")
+        self.assertEqual(conn.extra_data['ldap']['entrance_year'], '12')
 
     def test_disconnect_ldap(self):
         nu0 = User.objects.count()

setup.cfg

@@ -7,7 +7,6 @@ combine_as_imports = True
 default_section = THIRDPARTY
 include_trailing_comma = True
 known_allauth = allauth
-known_future_library = future,six
 known_django = django
 known_first_party = allauth_ens
 multi_line_output = 5