[sagenb] 34/179: some code refactoring

[felix salfelder]

This is an automated email from the git hooks/post-receive script.

felix-guest pushed a commit to branch master
in repository sagenb.

commit 356103bd48804e85a9c5b2f584be40066a4f88ec
Author: Robin Martinjak <rob at rmartinjak.de>
Date:   Fri Nov 2 14:02:50 2012 +0100

    some code refactoring
---
 sagenb/notebook/auth.py | 127 ++++++++++++++++++++++++++----------------------
 1 file changed, 70 insertions(+), 57 deletions(-)

diff --git a/sagenb/notebook/auth.py b/sagenb/notebook/auth.py
index 03550c1..f6fff51 100644
--- a/sagenb/notebook/auth.py
+++ b/sagenb/notebook/auth.py
@@ -24,17 +24,17 @@ class LdapAuth(AuthMethod):
     """
     Authentication via LDAP
 
-    User authentication:
-    1a. bind to LDAP with either
+    User authentication basically works like this:
+    1.1) bind to LDAP with either
             - generic configured DN and password (simple bind)
             - GSSAPI (e.g. Kerberos)
-    1b. find the ldap object matching username.
-        (return None if more than 1 object is found)
-    2. if 1 succeeds, try simple bind with the supplied user DN and password
+    1.2) find the ldap object matching username.
+
+    2) if 1 succeeds, try simple bind with the supplied user DN and password
 
     User lookup:
-    wildcard-search all configured "user lookup attributes" for
-    the given search string
+    wildcard-search all configured "user lookup attributes" for the given
+    search string
     """
 
     def _require_ldap(retval):
@@ -67,90 +67,103 @@ class LdapAuth(AuthMethod):
         import ldap
         from ldap.sasl import gssapi
         conn = ldap.initialize(self._conf['ldap_uri'])
+
+        if self._conf['ldap_gssapi']:
+            token = gssapi()
+            conn.sasl_interactive_bind_s('', token)
+        else:
+            conn.simple_bind_s(self._conf['ldap_binddn'], self._conf['ldap_bindpw'])
+
         try:
-            if self._conf['ldap_gssapi']:
-                token = gssapi()
-                conn.sasl_interactive_bind_s("", token)
-            else:
-                conn.simple_bind_s(self._conf['ldap_binddn'], self._conf['ldap_bindpw'])
-
-            result = conn.search_ext_s(self._conf['ldap_basedn'],
-                                         ldap.SCOPE_SUBTREE,
-                                         filterstr=query,
-                                         attrlist=attrlist,
-                                         timeout=self._conf['ldap_timeout'],
-                                         sizelimit=self._conf['ldap_sizelimit'])
-        except ldap.INVALID_CREDENTIALS:
-            raise ValueError, "invalid LDAP credentials"
-        except ldap.LDAPError, e:
-            raise ValueError, e
+            result = conn.search_ext_s(
+                    self._conf['ldap_basedn'],
+                    ldap.SCOPE_SUBTREE,
+                    filterstr=query,
+                    attrlist=attrlist,
+                    timeout=self._conf['ldap_timeout'],
+                    sizelimit=self._conf['ldap_sizelimit']
+                    )
+        except LDAPError, e:
+            print 'LDAP Error: %s' % str(e)
+            return []
         finally:
             conn.unbind_s()
+
         return result
 
     def _get_ldapuser(self, username, attrlist=None):
+        """
+        Returns a tuple containing the DN and a dict of attributes of the given
+        username, or (None, None) if the username is not found
+        """
         from ldap.filter import filter_format
-        try:
-            result = self._ldap_search(filter_format("(%s=%s)", [self._conf['ldap_username_attrib'], username]), attrlist)
-        except ValueError, e:
-            print(e)
-            return None
-        # return None if more than 1 object found
-        return result[0] if len(result) == 1 else None
+
+        result = self._ldap_search(filter_format('(%s=%s)', [self._conf['ldap_username_attrib'], username]), attrlist)
+
+        # only allow one unique result
+        # (len(result) will probably always be 0 or 1)
+        return result[0] if len(result) == 1 else (None, None)
 
     @_require_ldap(None)
     def user_lookup(self, search):
         from ldap.filter import filter_format
-        from ldap import LDAPError
+
+        uname_attrib = self._conf['ldap_username_attrib']
+        lookup_attribs = self._conf['ldap_lookup_attribs']
 
         # build ldap OR query
-        q = "(|%s)" % ''.join([filter_format("(%s=*%s*)", [a, search]) for a in self._conf['ldap_lookup_attribs']])
+        query = '(|%s)' % ''.join(filter_format('(%s=*%s*)', [a, search]) for a in lookup_attribs)
+
+        result = self._ldap_search(query, attrlist=[str(uname_attrib)])
 
-        try:
-            r = self._ldap_search(q, attrlist=[str(self._conf['ldap_username_attrib'])])
-        except ValueError, e:
-            print(e)
-            return []
-        except:
-            return []
         # return a list of usernames
-        return [x[1][self._conf['ldap_username_attrib']][0].lower() for x in r if x[1].has_key(self._conf['ldap_username_attrib'])]
+        unames = []
+        for dn, attribs in result:
+            uname_list = attribs.get(uname_attrib, None)
+            if uname_list:
+                # use only the first item of the attribute list
+                unames.append(uname_list[0])
+        return unames
 
     @_require_ldap(False)
     def check_user(self, username):
         # LDAP is NOT case sensitive while sage is, so only lowercase names are allowed
         if username != username.lower():
             return False
-        return self._get_ldapuser(username) is not None
+        dn, attribs = self._get_ldapuser(username)
+        return dn is not None
 
     @_require_ldap(False)
     def check_password(self, username, password):
         import ldap
-        # retrieve username's DN
-        try:
-            u = self._get_ldapuser(username)
-            #u[0] is DN, u[1] is a dict with all other attributes
-            userdn = u[0]
-        except ValueError:
+
+        dn, attribs = self._get_ldapuser(username)
+        if not dn:
             return False
 
-        # try to bind with that DN
+        # try to bind with found DN
         conn = ldap.initialize(uri=self._conf['ldap_uri'])
         try:
-            conn.simple_bind_s(userdn, password)
+            conn.simple_bind_s(dn, password)
             return True
         except ldap.INVALID_CREDENTIALS:
             return False
+        except ldap.LDAPError, e:
+            print 'LDAP Error: %s' % str(e)
+            return False
         finally:
             conn.unbind_s()
 
     @_require_ldap('')
     def get_attrib(self, username, attrib):
-        # translate some common attribute names to their ldap equivalents, i.e. "email" is "mail
-        attrib = 'mail' if attrib == 'email' else attrib
-
-        u = self._get_ldapuser(username)
-        if u is not None:
-            a = u[1][attrib][0] #if u[1].has_key(attrib) else ''
-            return a
-        return ''
+        # 'translate' attribute names used in ExtAuthUserManager to their ldap equivalents
+        # "email" is "mail"
+        if attrib == 'email'
+            attrib = 'mail'
+
+        dn, attribs = self._get_ldapuser(username, [attrib])
+        if not attribs:
+            return ''
+
+        # return the first item or '' if the attribute is missing
+        return attribs.get(attrib, [''])[0]

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/debian-science/packages/sagenb.git