[merge] backport stable fixes
authorAurelien Campeas <aurelien.campeas@logilab.fr>
Fri, 25 Jan 2013 14:33:40 +0100
changeset 8683 d537786e52b8
parent 8682 20bd1cdf86ae (current diff)
parent 8681 48731a0d3df8 (diff)
child 8684 6c7c2a02c9a0
[merge] backport stable fixes
server/ldaputils.py
server/test/unittest_ldapuser.py
sobjects/ldapparser.py
--- a/doc/book/en/admin/ldap.rst	Fri Jan 25 13:28:23 2013 +0100
+++ b/doc/book/en/admin/ldap.rst	Fri Jan 25 14:33:40 2013 +0100
@@ -12,49 +12,55 @@
 
 At cube creation time, one is asked if more sources are wanted. LDAP
 is one possible option at this time. Of course, it is always possible
-to set it up later in the `source` configuration file, which we
-discuss there.
+to set it up later using the `CWSource` entity type, which we discuss
+there.
 
 It is possible to add as many LDAP sources as wanted, which translates
-in as many [ldapxxx] sections in the `source` configuration file.
+in as many `CWSource` entities as needed.
 
 The general principle of the LDAP source is, given a proper
 configuration, to create local users matching the users available in
-the directory, deriving local user attributes from directory users
+the directory and deriving local user attributes from directory users
 attributes. Then a periodic task ensures local user information
 synchronization with the directory.
 
+Users handled by such a source should not be edited directly from
+within the application instance itself. Rather, updates should happen
+at the LDAP server level.
+
 Credential checks are _always_ done against the LDAP server.
 
-The base functionality for this is in
-:file:`cubicweb/server/sources/ldapuser.py`.
+.. Note::
 
-External dependencies
----------------------
-
-You'll need the following packages to make CubicWeb interact with your LDAP /
-Active Directory server:
+  There are currently two ldap source types: the older `ldapuser` and
+  the newer `ldapfeed`. The older will be deprecated anytime soon, as
+  the newer has now gained all the features of the old and does not
+  suffer from some of its illnesses.
 
-* python-ldap
-* ldaputils if using `ldapfeed` source
+  The ldapfeed creates real `CWUser` entities, and then
+  activate/deactivate them depending on their presence/absence in the
+  corresponding LDAP source. Their attribute and state
+  (activated/deactivated) are hence managed by the source mechanism;
+  they should not be altered by other means (as such alterations may
+  be overridden in some subsequent source synchronisation).
 
-Configurations options
-----------------------
 
-Let us enumerate the options (but please keep in mind that the
-authoritative source for these is in the aforementioned python
-module), by categories (LDAP server connection, LDAP schema mapping
-information, LDAP source internal configuration).
+Configurations options of an LDAPfeed source
+--------------------------------------------
+
+Let us enumerate the options by categories (LDAP server connection,
+LDAP schema mapping information).
 
 LDAP server connection options:
 
-* `host`, may contain port information using <host>:<port> notation.
-* `protocol`, choices are ldap, ldaps, ldapi
 * `auth-mode`, (choices are simple, cram_md5, digest_md5, gssapi, support
   for the later being partial as of now)
+
 * `auth-realm`, realm to use when using gssapi/kerberos authentication
+
 * `data-cnx-dn`, user dn to use to open data connection to the ldap (eg
   used to respond to rql queries)
+
 * `data-cnx-password`, password to use to open data connection to the
   ldap (eg used to respond to rql queries)
 
@@ -62,21 +68,29 @@
 leave data-cnx-dn and data-cnx-password empty. This is, however, quite
 unlikely in practice.
 
-LDAP schema mapping:
+LDAP schema mapping options:
 
 * `user-base-dn`, base DN to lookup for users
-* `user-scope`, user search scope
-* `user-classes`, classes of user
-* `user-attrs-map`, map from ldap user attributes to cubicweb attributes
-* `user-login-attr`, attribute used as login on authentication
+
+* `user-scope`, user search scope (valid values: "BASE", "ONELEVEL",
+  "SUBTREE")
 
-LDAP source internal configuration:
+* `user-classes`, classes of user (with Active Directory, you want to
+  say "user" here)
+
+* `user-filter`, additional filters to be set in the ldap query to
+  find valid users
+
+* `user-login-attr`, attribute used as login on authentication (with
+  Active Directory, you want to use "sAMAccountName" here)
 
 * `user-default-group`, name of a group in which ldap users will be by
   default. You can set multiple groups by separating them by a comma
-* `synchronization-interval`, interval between synchronization with the
-  ldap directory in seconds (default to once a day)
-* `cache-life-time`, life time of query cache in minutes (default to two hours).
+
+* `user-attrs-map`, map from ldap user attributes to cubicweb
+  attributes (with Active Directory, you want to use
+  sAMAccountName:login,mail:email,givenName:firstname,sn:surname)
+
 
 Other notes
 -----------
@@ -87,14 +101,13 @@
   authenticated but their status will not change (e.g. they will not
   be deactivated)
 
-* Changing the name of the ldap server in your script is fine, changing the base
-  DN isn't since it's used to identify already known users from others
+* The user-base-dn is a key that helps cubicweb map CWUsers to LDAP
+  users: beware updating it
 
 * When a user is removed from an LDAP source, it is deactivated in the
   CubicWeb instance; when a deactivated user comes back in the LDAP
   source, it (automatically) is activated again
 
-
 * You can use the :class:`CWSourceHostConfig` to have variants for a source
   configuration according to the host the instance is running on. To do so go on
   the source's view from the sources management view.
--- a/server/ldaputils.py	Fri Jan 25 13:28:23 2013 +0100
+++ b/server/ldaputils.py	Fri Jan 25 14:33:40 2013 +0100
@@ -38,6 +38,7 @@
 from ldapurl import LDAPUrl
 
 from cubicweb import ValidationError, AuthenticationError, Binary
+from cubicweb.server import utils
 from cubicweb.server.sources import ConnectionWrapper
 
 _ = unicode
@@ -336,7 +337,11 @@
         itemdict = {'dn': dn}
         for key, value in iterator:
             if self.user_attrs.get(key) == 'upassword': # XXx better password detection
-                itemdict[key] = Binary(value[0].encode('utf-8'))
+                value = value[0].encode('utf-8')
+                # we only support ldap_salted_sha1 for ldap sources, see: server/utils.py
+                if not value.startswith('{SSHA}'):
+                    value = utils.crypt_password(value)
+                itemdict[key] = Binary(value)
             else:
                 for i, val in enumerate(value):
                     value[i] = unicode(val, 'utf-8', 'replace')
--- a/server/test/data/ldap_test.ldif	Fri Jan 25 13:28:23 2013 +0100
+++ b/server/test/data/ldap_test.ldif	Fri Jan 25 14:33:40 2013 +0100
@@ -31,7 +31,7 @@
 gecos: Sylvain Thenault
 mail: sylvain.thenault@logilab.fr
 mail: syt@logilab.fr
-userPassword: {SSHA}v/8xJQP3uoaTBZz1T7Y0B3qOxRN1cj7D
+userPassword: syt
 
 dn: uid=adim,ou=People,dc=cubicweb,dc=test
 loginShell: /bin/bash
@@ -53,5 +53,5 @@
 gecos: Adrien Di Mascio
 mail: adim@logilab.fr
 mail: adrien.dimascio@logilab.fr
-userPassword: {SSHA}cPQOWqkkLDlfWFwxcl1m8V2JdySQBHfS
+userPassword: adim
 
--- a/server/test/data/slapd.conf.in	Fri Jan 25 13:28:23 2013 +0100
+++ b/server/test/data/slapd.conf.in	Fri Jan 25 14:33:40 2013 +0100
@@ -49,5 +49,5 @@
 rootdn          "cn=admin,dc=cubicweb,dc=test"
 rootpw          "cw"
 # Where the database file are physically stored for database #1
-directory       "%(apphome)s/ldapdb"
+directory       "%(testdir)s"
 
--- a/server/test/unittest_ldapuser.py	Fri Jan 25 13:28:23 2013 +0100
+++ b/server/test/unittest_ldapuser.py	Fri Jan 25 14:33:40 2013 +0100
@@ -23,6 +23,7 @@
 import time
 from os.path import join, exists
 import subprocess
+import tempfile
 
 from logilab.common.testlib import TestCase, unittest_main, mock_object, Tags
 
@@ -39,16 +40,13 @@
 
 def create_slapd_configuration(cls):
     global URL
+    slapddir = tempfile.mkdtemp('cw-unittest-ldap')
     config = cls.config
-    basedir = join(config.apphome, "ldapdb")
     slapdconf = join(config.apphome, "slapd.conf")
     confin = file(join(config.apphome, "slapd.conf.in")).read()
     confstream = file(slapdconf, 'w')
-    confstream.write(confin % {'apphome': config.apphome})
+    confstream.write(confin % {'apphome': config.apphome, 'testdir': slapddir})
     confstream.close()
-    if exists(basedir):
-        shutil.rmtree(basedir)
-    os.makedirs(basedir)
     # fill ldap server with some data
     ldiffile = join(config.apphome, "ldap_test.ldif")
     config.info('Initing ldap database')
@@ -69,6 +67,7 @@
         raise EnvironmentError('Cannot start slapd with cmdline="%s" (from directory "%s")' %
                                (" ".join(cmdline), os.getcwd()))
     URL = u'ldap://%s' % host
+    return slapddir
 
 def terminate_slapd(cls):
     config = cls.config
@@ -89,11 +88,32 @@
     def setUpClass(cls):
         from cubicweb.cwctl import init_cmdline_log_threshold
         init_cmdline_log_threshold(cls.config, cls.loglevel)
-        create_slapd_configuration(cls)
+        cls._tmpdir = create_slapd_configuration(cls)
 
     @classmethod
     def tearDownClass(cls):
         terminate_slapd(cls)
+        try:
+            shutil.rmtree(cls._tmpdir)
+        except:
+            pass
+
+class CheckWrongGroup(LDAPTestBase):
+
+    def test_wrong_group(self):
+        self.session.create_entity('CWSource', name=u'ldapuser', type=u'ldapfeed', parser=u'ldapfeed',
+                                   url=URL, config=CONFIG)
+        self.commit()
+        with self.session.repo.internal_session(safe=True) as session:
+            source = self.session.execute('CWSource S WHERE S type="ldapfeed"').get_entity(0,0)
+            config = source.repo_source.check_config(source)
+            # inject a bogus group here, along with at least a valid one
+            config['user-default-group'] = ('thisgroupdoesnotexists','users')
+            source.repo_source.update_config(source, config)
+            session.commit(free_cnxset=False)
+            # here we emitted an error log entry
+            stats = source.repo_source.pull_data(session, force=True, raise_on_error=True)
+            session.commit()
 
     def setUp(self):
         super(LDAPTestBase, self).setUp()
@@ -244,10 +264,6 @@
         source.pull_data(self.session)
         rset = self.sexecute('CWUser X WHERE X login %(login)s', {'login': 'syt'})
         self.assertEqual(len(rset), 1)
-        # test some password has been set
-        cu = self.session.system_sql('SELECT cw_upassword FROM cw_CWUser WHERE cw_eid=%s' % rset[0][0])
-        value = str(cu.fetchall()[0][0])
-        self.assertEqual(value, '{SSHA}v/8xJQP3uoaTBZz1T7Y0B3qOxRN1cj7D')
         self.assertTrue(self.repo.system_source.authenticate(
                 self.session, 'syt', password='syt'))
 
--- a/sobjects/ldapparser.py	Fri Jan 25 13:28:23 2013 +0100
+++ b/sobjects/ldapparser.py	Fri Jan 25 14:33:40 2013 +0100
@@ -130,8 +130,10 @@
         super(DataFeedLDAPAdapter, self).after_entity_copy(entity, sourceparams)
         if entity.__regid__ == 'EmailAddress':
             return
-        groups = [self._get_group(n) for n in self.source.user_default_groups]
-        entity.cw_set(in_group=groups)
+        groups = filter(None, [self._get_group(name)
+                               for name in self.source.user_default_groups])
+        if groups:
+            entity.cw_set(in_group=groups)
         self._process_email(entity, sourceparams)
 
     def is_deleted(self, extidplus, etype, eid):
@@ -172,5 +174,11 @@
 
     @cached
     def _get_group(self, name):
-        return self._cw.execute('Any X WHERE X is CWGroup, X name %(name)s',
-                                {'name': name}).get_entity(0, 0)
+        try:
+            return self._cw.execute('Any X WHERE X is CWGroup, X name %(name)s',
+                                    {'name': name}).get_entity(0, 0)
+        except IndexError:
+            self.error('group %r referenced by source configuration %r does not exist'
+                       % (name, self.source.uri))
+            return None
+