Merge lp:~ibeardslee/openobject-addons/users_ldap-tls into lp:openobject-addons

[Ian Beardslee]

Improved code for STARTTLS support in users_ldap

Brought the ldap_tls_s into res_company_ldap.connect()

Removed the 'Start TLS' from the tree view, to keep that a bit cleaner.

Once again, only tested against a LDAP server that requires STARTTLS.

[Stefan Rijnhart (Opener) (stefan-opener)]

Hi Ian,

thanks for resubmitting this. I have not seen this prerequisite branch functionality of bzr in action before, but it appears to be working pretty well apart from the phantom addition of 'hr_payroll/i18n/vi.po' that none of us commited in these branches (but that was added to the target branch in the meantime).

I am generally happy with your code changes and documentation, and have only one request. With regards to ldap.conf, you mention its default location and the fact that it may vary between distributions. It may be more correct to refer to its man page, which starts by explaining which files and locations are searched for LDAP defaults, in what order and how this can be manipulated using environment settings. The Sphynx syntax for this is

:manpage:`ldap.conf{5}`

Cheers,
Stefan.

review: Needs Fixing

=== modified file 'users_ldap/__openerp__.py'
--- users_ldap/__openerp__.py	2011-07-08 10:03:07 +0000
+++ users_ldap/__openerp__.py	2011-08-16 22:08:37 +0000
@@ -50,6 +50,11 @@
 LDAP account that is used to verify if a user exists before
 attempting to authenticate it.
 
+Securing the connection with STARTTLS is available for LDAP
+servers supporting STARTTLS. The default is to require STARTTLS.
+For further options configuring the LDAP settings, refer to the
+ldap.conf manpage :manpage:`ldap.conf(5)`.
+
 Security Considerations
 +++++++++++++++++++++++
 Users' LDAP passwords are never stored in the OpenERP database,
@@ -78,6 +83,9 @@
 always fails and the LDAP server is queried to do the
 authentication.
 
+Enabling STARTTLS ensures that the authentication query to the
+LDAP server is encrypted.
+
 User Template
 +++++++++++++
 In the LDAP configuration on the Company form, it is possible to
@@ -108,8 +116,12 @@
     "website" : "http://www.openerp.com",
     "category" : "Tools",
     "data" : [
+<<<<<<< TREE
         "users_ldap_view.xml",
         "user_ldap_installer.xml",
+=======
+        "users_ldap_view.xml",w
+>>>>>>> MERGE-SOURCE
     ],
     "active": False,
     "installable": True,

=== modified file 'users_ldap/users_ldap.py'
--- users_ldap/users_ldap.py	2011-08-16 22:08:34 +0000
+++ users_ldap/users_ldap.py	2011-08-16 22:08:37 +0000
@@ -50,7 +50,8 @@
             args = []
         cr.execute("""
             SELECT id, company, ldap_server, ldap_server_port, ldap_binddn,
-                   ldap_password, ldap_filter, ldap_base, "user", create_user
+                   ldap_password, ldap_filter, ldap_base, "user", create_user,
+                   ldap_tls
             FROM res_company_ldap
             WHERE ldap_server != '' """ + id_clause + """ ORDER BY sequence
         """, args)
@@ -67,7 +68,11 @@
 
         uri = 'ldap://%s:%d' % (conf['ldap_server'],
                                 conf['ldap_server_port'])
-        return ldap.initialize(uri)
+
+        connection = ldap.initialize(uri)
+        if conf['ldap_tls']:
+            connection.start_tls_s()
+        return connection
 
     def authenticate(self, conf, login, password):
         """
@@ -211,12 +216,15 @@
             help="Model used for user creation"),
         'create_user': fields.boolean('Create user',
             help="Create the user if not in database"),
+        'ldap_tls': fields.boolean('Use TLS',
+            help="Use STARTTLS to connect to the LDAP server"),
     }
     _defaults = {
         'ldap_server': '127.0.0.1',
         'ldap_server_port': 389,
         'sequence': 10,
         'create_user': True,
+        'ldap_tls': True,
     }
 
 CompanyLDAP()

=== modified file 'users_ldap/users_ldap_view.xml'
--- users_ldap/users_ldap_view.xml	2011-06-11 15:18:53 +0000
+++ users_ldap/users_ldap_view.xml	2011-08-16 22:08:37 +0000
@@ -20,6 +20,7 @@
                             <field name="user"/>
                             <newline/>
                             <field name="sequence"/>
+                            <field name="ldap_tls"/>
                         </form>
                         <tree string="LDAP Configuration">
                             <field name="sequence"/>