LDAP: upodate user attr with ldap attr

tracim/test.ini

@@ -31,7 +31,7 @@ ldap_base_dn = dc=directory,dc=fsf,dc=org
 ldap_bind_dn = cn=admin,dc=directory,dc=fsf,dc=org
 ldap_bind_pass = toor
 ldap_ldap_naming_attribute = uid
-ldap_user_attributes = mail=email
+ldap_user_attributes = mail=email,pubname=display_name
 ldap_tls = False
 ldap_group_enabled = False
 use = config:development.ini

tracim/tracim/fixtures/ldap.py

@@ -40,7 +40,8 @@ ldap_test_server_fixtures = {
             'attributes': {
                 'uid': 'richard-not-real-email@fsf.org',
                 'userPassword': 'rms',
-                'mail': 'richard-not-real-email@fsf.org'
+                'mail': 'richard-not-real-email@fsf.org',
+                'pubname': 'Richard Stallman',
             }
         },
         {
@@ -49,7 +50,8 @@ ldap_test_server_fixtures = {
             'attributes': {
                 'uid': 'lawrence-not-real-email@fsf.org',
                 'userPassword': 'foobarbaz',
-                'mail': 'lawrence-not-real-email@fsf.org'
+                'mail': 'lawrence-not-real-email@fsf.org',
+                'pubname': 'Lawrence Lessig',
             }
         },
     ]

tracim/tracim/lib/auth/ldap.py

@@ -1,4 +1,5 @@
 # -*- coding: utf-8 -*-
+import transaction
 from tg.configuration.auth import TGAuthMetadata
 from who_ldap import LDAPAttributesPlugin as BaseLDAPAttributesPlugin
 from who_ldap import LDAPGroupsPlugin as BaseLDAPGroupsPlugin
@@ -92,19 +93,27 @@ class LDAPSearchAuthenticatorPlugin(BaseLDAPSearchAuthenticatorPlugin):
         # Note: super().authenticate return None if already authenticated or not found
         email = super().authenticate(environ, identity)
         if email:
-            self._sync_ldap_user(email)
+            self._sync_ldap_user(email, environ, identity)
         return email
 
-    def _sync_ldap_user(self, email):
+    def _sync_ldap_user(self, email, environ, identity):
+        # Create or get user for connected email
         if not self._user_api.user_with_email_exists(email):
             user = User(email=email, imported_from=LDAPAuth.name)
             DBSession.add(user)
-            import transaction
-            transaction.commit()
+        else:
+            user = self._user_api.get_one_by_email(email)
+
+        # Retrieve ldap user attributes
+        self._auth.ldap_user_provider.add_metadata_for_auth(environ, identity)
 
-            # TODO - B.S. - 20160208: Voir avec Damien, si je ne fait pas de transaction.commit()
-            # manuellement la donnée n'est pas en base.
-            # self._user_api.create_user(email=email, save_now=True)
+        # Update user with ldap attributes
+        user_ldap_values = identity.get('user').copy()
+        for field_name in user_ldap_values:
+            setattr(user, field_name, user_ldap_values[field_name])
+
+        DBSession.flush()
+        transaction.commit()
 
 
 class LDAPApplicationAuthMetadata(TGAuthMetadata):
@@ -156,9 +165,11 @@ class LDAPAttributesPlugin(BaseLDAPAttributesPlugin):
         self._user_api = UserApi(None)
 
     def add_metadata(self, environ, identity):
+        # We disable metadata recuperation, we do it at connection in LDAPSearchAuthenticatorPlugin._sync_ldap_user
+        return
+
+    def add_metadata_for_auth(self, environ, identity):
         super().add_metadata(environ, identity)
-        # TODO - B.S. - 20160212: identity contains now som information from LDAP what we can save in local database
-        identity[self.name] = self._user_api.get_one_by_email(identity.get('repoze.who.userid'))
 
     @property
     def local_fields(self):

tracim/tracim/tests/__init__.py

@@ -1,10 +1,12 @@
 # -*- coding: utf-8 -*-
 """Unit and functional test suite for tracim."""
 import argparse
+import os
 from os import getcwd
 
 import ldap3
 import tg
+import time
 import transaction
 from gearbox.commands.setup_app import SetupAppCommand
 from ldap_test import LdapServer
@@ -20,7 +22,7 @@ from sqlalchemy.schema import Sequence
 from sqlalchemy.schema import Table
 from tg import config
 from tg.util import Bunch
-from webtest import TestApp
+from webtest import TestApp as BaseTestApp, AppError
 from who_ldap import make_connection
 
 from tracim.command import BaseCommand
@@ -32,6 +34,21 @@ __all__ = ['setup_app', 'setup_db', 'teardown_db', 'TestController']
 application_name = 'main_without_authn'
 
 
+class TestApp(BaseTestApp):
+    def _check_status(self, status, res):
+        """ Simple override to print html content when error"""
+        try:
+            super()._check_status(status, res)
+        except AppError as exc:
+            dump_file_path = "/tmp/debug_%d_%s.html" % (time.time() * 1000, res.request.path_qs[1:])
+            if os.path.exists("/tmp"):
+                with open(dump_file_path, 'w') as dump_file:
+                    print(res.ubody, file=dump_file)
+                # Update exception message with info about this dumped file
+                exc.args = ('%s html error file dump in %s' % (exc.args[0], dump_file_path), ) + exc.args[1:]
+            raise exc
+
+
 def load_app(name=application_name):
     """Load the test application."""
     return TestApp(loadapp('config:test.ini#%s' % name, relative_to=getcwd()))

tracim/tracim/tests/functional/test_ldap_authentication.py

@@ -3,7 +3,7 @@
 Integration tests for the ldap authentication sub-system.
 """
 from tracim.fixtures.ldap import ldap_test_server_fixtures
-from nose.tools import eq_
+from nose.tools import eq_, ok_
 
 from tracim.model import DBSession, User
 from tracim.tests import LDAPTest, TracimTestController
@@ -39,3 +39,16 @@ class TestAuthentication(LDAPTest, TracimTestController):
 
         # User is registered in tracim database
         eq_(1, DBSession.query(User).filter(User.email == 'richard-not-real-email@fsf.org').count())
+
+    def test_ldap_attributes_sync(self):
+        # User is already know in database
+        eq_(1, DBSession.query(User).filter(User.email == 'lawrence-not-real-email@fsf.org').count())
+
+        # His display name is Lawrence L.
+        lawrence = DBSession.query(User).filter(User.email == 'lawrence-not-real-email@fsf.org').one()
+        eq_('Lawrence L.', lawrence.display_name)
+
+        # After connexion with LDAP, his display_name is updated (see ldap fixtures)
+        self._connect_user('lawrence-not-real-email@fsf.org', 'foobarbaz')
+        lawrence = DBSession.query(User).filter(User.email == 'lawrence-not-real-email@fsf.org').one()
+        eq_('Lawrence Lessig', lawrence.display_name)

tracim/tracim/tests/functional/test_ldap_restrictions.py

@@ -59,7 +59,7 @@ class TestAuthentication(LDAPTest, TracimTestController):
 
         # If we force edit of user, "email" field will be not updated
         eq_(lawrence.email, 'lawrence-not-real-email@fsf.org')
-        eq_(lawrence.display_name, 'Lawrence Lessig')
+        eq_(lawrence.display_name, 'Lawrence L.')
 
         try_post_user = self.app.post(
             '/user/%d?_method=PUT' % lawrence.user_id,

tracim/tracim/websetup/bootstrap.py

@@ -43,7 +43,7 @@ def bootstrap(command, conf, vars):
         # TODO: - B.S. - 20160212: Following fixture is LDAP tests specific, should make an little fixture management
         # for tests
         lawrence = model.User()
-        lawrence.display_name = 'Lawrence Lessig'
+        lawrence.display_name = 'Lawrence L.'
         lawrence.email = 'lawrence-not-real-email@fsf.org'
         lawrence.password = 'foobarbaz'
         model.DBSession.add(lawrence)