Split try/except to raise appropriate warnings on bind failures.

Added unbind calls in failure conditions.

1 files changed, 58 insertions, 46 deletions

diff --git a/Authenticators/LDAP/LDAPauth.py b/Authenticators/LDAP/LDAPauth.py
index 7277cd0..0dd31de 100644
--- a/Authenticators/LDAP/LDAPauth.py
+++ b/Authenticators/LDAP/LDAPauth.py
@@ -426,66 +426,78 @@ def do_main_program():
             FALL_THROUGH = -2
             AUTH_REFUSED = -1
             
+            # SuperUser is a special login.
             if name == 'SuperUser':
                 debug('Forced fall through for SuperUser')
                 return (FALL_THROUGH, None, None)
             
-            #Otherwise, let's check the LDAP server
+            # Otherwise, let's check the LDAP server.
             uid = None
-            try:
-                #Attempt to bind to LDAP server with user-provided credentials
-                ldap_conn = ldap.initialize(cfg.ldap.ldap_uri, 0)
-                if cfg.ldap.bind_dn:
-                    bind_dn = cfg.ldap.bind_dn
-                    bind_pass = cfg.ldap.bind_pass
+            ldap_conn = ldap.initialize(cfg.ldap.ldap_uri, 0)
+            if cfg.ldap.bind_dn:
+                # Bind the functional account to search the directory.
+                bind_dn = cfg.ldap.bind_dn
+                bind_pass = cfg.ldap.bind_pass
+                try:
+                    ldap_conn.bind_s(bind_dn, bind_pass)
+                except ldap.INVALID_CREDENTIALS: 
+                    ldap_conn.unbind()
+                    warning("Invalid credentials for bind_dn=" + bind_dn)
+                    return (AUTH_REFUSED, None, None)
+            else:
+                # Bind the user account to search the directory.
+                bind_dn = "%s=%s,%s" % (cfg.ldap.username_attr, name, cfg.ldap.users_dn)
+                bind_pass = pw
+                try:
+                    ldap_conn.bind_s(bind_dn, bind_pass)
+                except ldap.INVALID_CREDENTIALS: 
+                    ldap_conn.unbind()
+                    warning("User " + name + " failed with wrong password")
+                    return (AUTH_REFUSED, None, None)
+
+            # Search for the user.
+            res = ldap_conn.search_s(cfg.ldap.users_dn, ldap.SCOPE_SUBTREE, '(%s=%s)' % (cfg.ldap.username_attr, name), [cfg.ldap.number_attr, cfg.ldap.display_attr])
+            if len(res) == 0:
+                warning("User " + name + " not found")
+                if cfg.user.reject_on_miss:
+                    return (AUTH_REFUSED, None, None)
                 else:
-                    bind_dn = "%s=%s,%s" % (cfg.ldap.username_attr, name, cfg.ldap.users_dn)
-                    bind_pass = pw
-                ldap_conn.bind_s(bind_dn, bind_pass)
-                res = ldap_conn.search_s(cfg.ldap.users_dn, ldap.SCOPE_SUBTREE, '(%s=%s)' % (cfg.ldap.username_attr, name), [cfg.ldap.number_attr, cfg.ldap.display_attr])
-                if len(res) == 0:
-                    warning("User " + name + " not found")
-                    if cfg.user.reject_on_miss:
-                        return (AUTH_REFUSED, None, None)
-                    else:
-                        return (FALL_THROUGH, None, None)
-                match = res[0] #Only interested in the first result, as there should only be one match
+                    return (FALL_THROUGH, None, None)
+            match = res[0] #Only interested in the first result, as there should only be one match
                 
-                #Parse the user information
-                uid = int(match[1][cfg.ldap.number_attr][0])
-                displayName = match[1][cfg.ldap.display_attr][0]
-                debug('User match found, display "' + displayName + '" with UID ' + repr(uid))
+            # Parse the user information.
+            uid = int(match[1][cfg.ldap.number_attr][0])
+            displayName = match[1][cfg.ldap.display_attr][0]
+            debug('User match found, display "' + displayName + '" with UID ' + repr(uid))
                 
-                #Optionally check groups
-                if cfg.ldap.group_cn != "" :
-                    debug('Checking group membership for ' + name)
+            # Optionally check groups.
+            if cfg.ldap.group_cn != "" :
+                debug('Checking group membership for ' + name)
                     
-                    #Search for user in group
-                    res = ldap_conn.search_s(cfg.ldap.group_cn, ldap.SCOPE_SUBTREE, '(%s=%s=%s,%s)' % (cfg.ldap.group_attr, cfg.ldap.username_attr, name, cfg.ldap.users_dn), [cfg.ldap.number_attr, cfg.ldap.display_attr])
+                #Search for user in group
+                res = ldap_conn.search_s(cfg.ldap.group_cn, ldap.SCOPE_SUBTREE, '(%s=%s=%s,%s)' % (cfg.ldap.group_attr, cfg.ldap.username_attr, name, cfg.ldap.users_dn), [cfg.ldap.number_attr, cfg.ldap.display_attr])
                     
-                    # Check if the user is a member of the group
-                    if len(res) < 1:
-                        debug('User ' + name + ' failed with no group membership')
-                        return (AUTH_REFUSED, None, None)
+                # Check if the user is a member of the group
+                if len(res) < 1:
+                    debug('User ' + name + ' failed with no group membership')
+                    return (AUTH_REFUSED, None, None)
                     
-                # Second bind to test user credentials if using bind_dn.
-                if cfg.ldap.bind_dn:
-                    bind_dn = "%s=%s,%s" % (cfg.ldap.username_attr, name, cfg.ldap.users_dn)
-                    bind_pass = pw
+            # Second bind to test user credentials if using bind_dn.
+            if cfg.ldap.bind_dn:
+                bind_dn = "%s=%s,%s" % (cfg.ldap.username_attr, name, cfg.ldap.users_dn)
+                bind_pass = pw
+                try:
                     ldap_conn.bind_s(bind_dn, bind_pass)
+                except ldap.INVALID_CREDENTIALS: 
+                    ldap_conn.unbind()
+                    warning("User " + name + " failed with wrong password")
+                    return (AUTH_REFUSED, None, None)
 
-                #Unbind and close connection
-                ldap_conn.unbind()
+            # Unbind and close connection.
+            ldap_conn.unbind()
                 
-            #What follows below are various what-if scenarios: authentication failures and successes
-                     
-            #LDAP bind failed - expected to happen if bad login
-            except ldap.INVALID_CREDENTIALS: 
-                warning("User " + name + " failed with wrong password")
-                return (AUTH_REFUSED, None, None)
-    
-            #If we get here, the login is correct.
-            #Add the user/id combo to cache, then accept:
+            # If we get here, the login is correct.
+            # Add the user/id combo to cache, then accept:
             self.name_uid_cache[displayName] = uid
             debug("Login accepted for " + name)
             return (uid + cfg.user.id_offset, displayName, [])