Fixes from Elarnon's review

README.rst

@@ -183,7 +183,7 @@ Auto-signup
     - extra_data in SociallAccount instance, containing these field, plus *annee* and *promotion* parsed from LDAP's *homeDirectory* field (available only on first connection)
       
 Account deprecation
-  At the beginning of each year (i.e. early November), to prevent clipper username conflicts, you should run ``$ python manage.py deprecate_clippers``. Every association clipper username <-> user will then be set on hold, and at the first subsequent connection, a verification of the account will be made (using LDAP), so that a known user keeps his account, but a newcomer won't inherit an archicube's.
+  At the beginning of each year (i.e. early November), to prevent clipper username conflicts, you should run ``$ python manage.py deprecate_clippers``. Every association clipper username <-> user will then be put on hold, and at the first subsequent connection, a verification of the account will be made (using LDAP), so that a known user keeps his account, but a newcomer won't inherit an archicube's.
 
 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.

allauth_ens/adapter.py

@@ -12,18 +12,18 @@ User = get_user_model()
 
 import six
 
-DEPARTMENTS_LIST = (
+DEPARTMENTS_LIST = {
-    ('phy', u'Physique'),
+    'phy': u'Physique',
-    ('maths', u'Maths'),
+    'maths': u'Maths',
-    ('bio', u'Biologie'),
+    'bio': u'Biologie',
-    ('chimie', u'Chimie'),
+    'chimie': u'Chimie',
-    ('geol', u'Géosciences'),
+    'geol': u'Géosciences',
-    ('dec', u'DEC'),
+    'dec': u'DEC',
-    ('info', u'Informatique'),
+    'info': u'Informatique',
-    ('litt', u'Littéraire'),
+    'litt': u'Littéraire',
-    ('guests', u'Pensionnaires étrangers'),
+    'guests': u'Pensionnaires étrangers',
-    ('pei', u'PEI'),
+    'pei': u'PEI',
-)
+}
 
 def _init_ldap():
     server = getattr(settings, "LDAP_SERVER", "ldaps://ldap.spi.ens.fr:636")
@@ -41,7 +41,7 @@ def _init_ldap():
 
 def _extract_infos_from_ldap(infos, data={}):
     # Name
-    data['name'] = infos.get('cn', [''])[0].decode("utf-8")
+    data['name'] = infos.get('cn', [b''])[0].decode("utf-8")
 
     # Parsing homeDirectory to get entrance year and departments
     annee = '00'
@@ -49,17 +49,18 @@ def _extract_infos_from_ldap(infos, data={}):
 
     if 'homeDirectory' in infos:
         dirs = infos['homeDirectory'][0].split('/')
-        if dirs[1] == 'users':
+        if len(dirs) >= 4 and dirs[1] == 'users':
+            # Assume template "/users/<year>/<department>/clipper/"
             annee = dirs[2]
             dep = dirs[3]
-            dep = dict(DEPARTMENTS_LIST).get(dep.lower(), '')
+            dep = DEPARTMENTS_LIST.get(dep.lower(), '')
             promotion = u'%s %s' % (dep, annee)
             data['annee'] = annee
             data['promotion'] = promotion
 
     # Mail
     pmail = infos.get('mailRoutingAddress', [])
-    if len(pmail) > 0 :
+    if pmail:
         data['email'] = pmail[0]
     return data
 
@@ -72,9 +73,9 @@ def get_ldap_infos(clipper):
         info = l.search_s('dc=spi,dc=ens,dc=fr',
                           ldap.SCOPE_SUBTREE,
                           ('(uid=%s)' % (clipper,)),
-                          [str("cn"),
+                          ['cn',
-                           str("mailRoutingAddress"),
+                           'mailRoutingAddress',
-                           str("homeDirectory") ])
+                           'homeDirectory' ])
 
         if len(info) > 0:
             data = _extract_infos_from_ldap(info[0][1], data)
@@ -93,49 +94,50 @@ class LongTermClipperAccountAdapter(DefaultSocialAccountAdapter):
 
     def pre_social_login(self, request, sociallogin):
         if sociallogin.account.provider != "clipper":
-            return Super(LongTermClipperAccountAdapter, self).pre_social_login(request, sociallogin)
+            return super(LongTermClipperAccountAdapter, self).pre_social_login(request, sociallogin)
         
         clipper = sociallogin.account.uid
         try:
             a = SocialAccount.objects.get(provider='clipper_inactive',
                                           uid=clipper)
-            # An account with that uid was registered, but potentially
-            # deprecated at the beginning of the year
-            # We need to check that the user is still the same as before
-            ldap_data = get_ldap_infos(clipper)
-            self._ldap_data = ldap_data
-            
-            if a.user.username != self.get_username(clipper, ldap_data):
-                # The admission year is different
-                # We need a new SocialAccount
-                # But before that, we need to invalidate the email address of
-                # the previous user
-                email = ldap_data.get('email')
-                u_mails = EmailAddress.objects.filter(user=a.user)
-                try:
-                    clipper_mail = u_mails.get(email=email)
-                    if clipper_mail.primary:
-                        n_mails = u_mails.filter(primary=False)
-                        if n_mails.exists():
-                            n_mails[0].set_as_primary()
-                        else:
-                            user_email(a.user, '')
-                            a.user.save()
-                    clipper_mail.delete()
-                except EmailAddress.DoesNotExist:
-                    pass
-                return
-            
-            # The admission year is the same, we can update the model and keep
-            # the previous SocialAccount instance
-            a.provider = 'clipper'
-            a.save()
-            
-            # Redo the thing that had failed just before
-            sociallogin.lookup()
-            
         except SocialAccount.DoesNotExist:
             return
+        
+        # An account with that uid was registered, but potentially
+        # deprecated at the beginning of the year
+        # We need to check that the user is still the same as before
+        ldap_data = get_ldap_infos(clipper)
+        sociallogin._ldap_data = ldap_data
+
+        if a.user.username != self.get_username(clipper, ldap_data):
+            # The admission year is different
+            # We need a new SocialAccount
+            # But before that, we need to invalidate the email address of
+            # the previous user
+            email = ldap_data.get('email')
+            u_mails = EmailAddress.objects.filter(user=a.user)
+            try:
+                clipper_mail = u_mails.get(email=email)
+                if clipper_mail.primary:
+                    n_mails = u_mails.filter(primary=False)
+                    if n_mails.exists():
+                        n_mails[0].set_as_primary()
+                    else:
+                        user_email(a.user, '')
+                        a.user.save()
+                clipper_mail.delete()
+            except EmailAddress.DoesNotExist:
+                pass
+            return
+
+        # The admission year is the same, we can update the model and keep
+        # the previous SocialAccount instance
+        a.provider = 'clipper'
+        a.save()
+
+        # Redo the thing that had failed just before
+        sociallogin.lookup()
+            
 
     def get_username(self, clipper, data):
         """
@@ -151,7 +153,7 @@ class LongTermClipperAccountAdapter(DefaultSocialAccountAdapter):
         user.set_unusable_password()
         
         clipper = sociallogin.account.uid
-        ldap_data = self._ldap_data if hasattr(self, '_ldap_data') \
+        ldap_data = sociallogin._ldap_data if hasattr(sociallogin, '_ldap_data') \
                     else get_ldap_infos(clipper)
         
         username = self.get_username(clipper, ldap_data)
@@ -182,7 +184,7 @@ def deprecate_clippers():
     clippers = SocialAccount.objects.filter(provider='clipper')
     c_uids = clippers.values_list('uid', flat=True)
     
-    # Clear old clipper accounts that wer replaced by new ones (o avoid conflicts)
+    # Clear old clipper accounts that were replaced by new ones (to avoid conflicts)
     SocialAccount.objects.filter(provider='clipper_inactive', uid__in=c_uids).delete()
     
     # Deprecate accounts