Re: [GMG-Devel] LDAP Integration

[Christopher Allan Webber]

Sebastian Hugentobler writes:

> Hi all
>
> I patched the ldap integration for my needs and now I'd like to know if
> there is interest to merge it back. As I am inexperienced with ldap it
> would not hurt if someone with more knowledge could take a look at the
> changes :) (especially concerning security implications of which I am
> ignorant).

I've Cc'ed Rodney, who did the original authentication work.  Do you
have thoughts, Rodney, if you could spare the time?

I'm very unfamliar with LDAP so it would not be wise for me to make this
call.

> There are some new config keys:
>
> - LDAP_USER_ATTRIBUTE: was hardcoded as *uid*, should fix issue #924 (I
> hope I got the correct issue). Mandatory.

This looks important... we should probably merge as its own patch?

> - LDAP_UID_SEARCH_FIELD: take the username from this attribute. I needed
> this when I used email addresses as login names. Optional.

> - LDAP_USER_FILTER: an ldap filter. I use this to determine group
> membership. Optional.

There's a bug in the code here:

+                use_user_filter = v['LDAP_USER_FILTER'] is not None
+                user_allowd = True
[...]
+                return (uid, email) if user_allowed else (False, None) 

If use_user_filter is False, this code will error out...

Otherwise this looks good from *my* reading, but I don't actually know
what's going on.

> How I am using it: I've got an openldap server which I use for owncloud,
> email, git and prosody authentication (and now for mediagoblin too).
> Whether a user has access to one of the services is determined by group
> membership.

Interesting!  Glad to hear people getting such cool usage in!

> Thanks for your thoughts,
>
> Sebastian

As an aside, this is the type of thing that normally we should do on the
issue tracker, but full ack that it's pretty borked right now if you
don't already have an account there.... we might be able to fix that
shortly.  Do you currently have an account on the tracker?

>From 414d4d2c84fd726b6fbf68a72d521ff2f4c32abd Mon Sep 17 00:00:00 2001
From: Sebastian Hugentobler <address@hidden>
Date: Fri, 13 Nov 2015 10:35:18 +0100
Subject: [PATCH] allow more ldap integration

New config key *LDAP_USER_ATTRIBUTE*, fixes #924.

Adds the possibility to take the username from a defined attribute in
the directory (like the mail attribute already did). Set the
*LDAP_UID_SEARCH_FIELD* config value to use this.

Adds the possibility to filter users (eg. with groups). Set the
*LDAP_USER_FILTER* to use this.
---
 mediagoblin/plugins/ldap/tools.py | 37 ++++++++++++++++++++++++++++---------
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/mediagoblin/plugins/ldap/tools.py 
b/mediagoblin/plugins/ldap/tools.py
index 2be2dcd..275fbc2 100644
--- a/mediagoblin/plugins/ldap/tools.py
+++ b/mediagoblin/plugins/ldap/tools.py
@@ -35,18 +35,24 @@ class LDAP(object):
             _log.info('Initiating TLS')
             self.conn.start_tls_s()
 
-    def _get_email(self, server, username):
+    def _get_attribute(self, server, username, attribute_id):
         try:
             results = self.conn.search_s(server['LDAP_SEARCH_BASE'],
-                                        ldap.SCOPE_SUBTREE, 'uid={0}'
-                                        .format(username),
-                                        [server['EMAIL_SEARCH_FIELD']])
+                                        ldap.SCOPE_SUBTREE, '{0}={1}'
+                                        .format(server['LDAP_USER_ATTRIBUTE'], 
username),
+                                        [server[attribute_id]])
 
-            email = results[0][1][server['EMAIL_SEARCH_FIELD']][0]
+            attribute_value = results[0][1][server[attribute_id]][0]
         except KeyError:
-            email = None
+            attribute_value = None
+
+        return attribute_value
+
+    def _get_email(self, server, username):
+        return self._get_attribute(server, username, 'EMAIL_SEARCH_FIELD')
 
-        return email
+    def _get_uid(self, server, username):
+         return self._get_attribute(server, username, 'LDAP_UID_SEARCH_FIELD')
 
     def login(self, username, password):
         for k, v in six.iteritems(self.ldap_settings):
@@ -54,8 +60,21 @@ class LDAP(object):
                 self._connect(v)
                 user_dn = v['LDAP_USER_DN_TEMPLATE'].format(username=username)
                 self.conn.simple_bind_s(user_dn, password.encode('utf8'))
+
                 email = self._get_email(v, username)
-                return username, email
+                uid = self._get_uid(v, username) if v['LDAP_UID_SEARCH_FIELD'] 
is not None else username
+
+                use_user_filter = v['LDAP_USER_FILTER'] is not None
+                user_allowd = True
+
+                if use_user_filter:
+                    user_allowed = len(self.conn.search_s(
+                                v['LDAP_SEARCH_BASE'],
+                                ldap.SCOPE_SUBTREE,
+                                
v['LDAP_USER_FILTER'].format(username=username))) == 1
+
+
+                return (uid, email) if user_allowed else (False, None) 
 
             except ldap.LDAPError, e:
                 _log.info(e)
@@ -63,5 +82,5 @@ class LDAP(object):
             finally:
                 _log.info('Unbinding {0}.'.format(v['LDAP_SERVER_URI']))
                 self.conn.unbind()
-
         return False, None
+
-- 
2.6.3