Commits

farkas  committed 420cf49

Issue number: USER-61

Jeff Turner's improvements to the LDAPCredentialsProvider, which
drastically improve logging when something goes wrong.

Thanks Jeff!

  • Participants
  • Parent commits d867514

Comments (0)

Files changed (1)

File src/java/com/opensymphony/user/provider/ldap/LDAPCredentialsProvider.java

 
 import java.util.*;
 
+import javax.naming.AuthenticationException;
+import javax.naming.Context;
 import javax.naming.NamingEnumeration;
 import javax.naming.NamingException;
 import javax.naming.directory.*;
 
 
 /**
- * DOCUMENT ME!
- *
- * @author $author$
- * @version $Revision: 1.2 $
+ * Provider for checking credentials against a LDAP directory.
+ * <p>
+ * Tries to connect to an LDAP directory with the specified username/password. Succeeds or fails depending on whether the
+ * LDAP authentication succeeds/fails.
+ * <p>
+ * The authentication algorithm is as follows:
+ * <ul>
+ *   <li>Establish an anonymous or authenticated connection with LDAP
+ *   <li>Search within a subtree for a node representing the user, eg. search below 'ou=People,dc=example,dc=com' for (uid=fred) if the username is 'fred'.
+ *   <li>If a user node is found (eg. 'uid=fred,ou=People,dc=example,dc=com'), try to connect to LDAP again, using the
+ * found node as the 'bind DN', and using the user's password.
+ *   <li>If this second connection succeeds, return true
+ * </ul>
+ * <p>A sample osuser.xml configuration:
+ * <pre>
+ * &lt;provider class="com.opensymphony.user.provider.ldap.LDAPCredentialsProvider">
+     &lt;property name="java.naming.factory.initial">com.sun.jndi.ldap.LdapCtxFactory&lt;/property>
+     &lt;property name="java.naming.provider.url">ldap://localhost:389&lt;/property>
+     &lt;property name="searchBase">ou=People,dc=example,dc=com&lt;/property>
+     &lt;property name="uidSearchName">uid&lt;/property>
+     &lt;property name="java.naming.security.principal">cn=admin,dc=example,dc=com&lt;/property>
+     &lt;property name="java.naming.security.credentials">secret&lt;/property>
+     &lt;property name="exclusive-access">true&lt;/property>
+    &lt;/provider>
+  * </pre>
+ * The security principal and credentials lines are optional, depending on whether your initial connection need be authenticated or not.
+ * <p>
+ * Currently, there must be <strong>at least one other non-LDAP CredentialsProvider</strong> configured for this provider to work.
+ * This is because there are user management features that LDAPCredentialsProvider does not provide on its own (create/delete user, change password).
+ * When calls to these methods are made, LDAPCredentialsProvider delegates the call to the other CredentialsProvider implementation.
+ * <p>
+ * Notes:
+ * <ul>
+ *   <li>Entering blank password will always fail, regardless of whether the underlying LDAP allows anonymous user connects.
+ *   <li>If the initial LDAP connection cannot be established, or there is an unexpected error, the authentication attempt
+ * is passed on to other non-LDAP CredentialsProvider.
+ *   <li>If the user exists in LDAP but the password was incorrect, the module fails without consulting other CredentialsProviders.
+ *   <li>Turning logging up to DEBUG will reveal details on authentication attempts.
+ * </ul>
+ *  
+ * @author <a href="mailto:jeff@atlassian.com">Jeff Turner</a> 
  */
 public class LDAPCredentialsProvider implements CredentialsProvider {
     //~ Static fields/initializers /////////////////////////////////////////////
         TimeAndPassword tp = (TimeAndPassword) cache.get(name);
 
         if ((tp != null) && tp.password.equals(password) && (tp.time > System.currentTimeMillis())) {
+            if (log.isDebugEnabled()) {
+                log.debug("Successful authentication for " + name + " from cached LDAP lookup");
+            }
+
             return true;
         }
 
+        DirContext ctx = null;
+
+        try {
+            ctx = new InitialDirContext(env);
+        } catch (NamingException e) {
+            log.error("Could not connect to LDAP. Please check your " + "host ('" + env.get(Context.PROVIDER_URL) + "'), " + "bind DN ('" + env.get(Context.SECURITY_PRINCIPAL) + "') and bind password.", e);
+
+            return tryOtherCredentialsProviders(name, password);
+        }
+
+        StringBuffer filterBuffer = new StringBuffer(uidSearchName).append("=").append(name);
+
+        String[] attrIDs = {uidSearchName};
+        SearchControls ctls = new SearchControls();
+        ctls.setReturningAttributes(attrIDs);
+        ctls.setSearchScope(SearchControls.SUBTREE_SCOPE);
+
+        if (log.isDebugEnabled()) {
+            log.debug("Doing initial search: " + "username='" + env.get(Context.SECURITY_PRINCIPAL) + "', password='" + env.get(Context.SECURITY_CREDENTIALS) + "', base='" + searchBase + "', filter='" + filterBuffer + "'");
+        }
+
+        NamingEnumeration results = null;
+
+        try {
+            results = ctx.search(searchBase, filterBuffer.toString(), ctls);
+        } catch (NamingException e) {
+            log.error("Connected to LDAP, but could not perform " + (env.containsKey(Context.SECURITY_PRINCIPAL) ? "authenticated" : "anonymous") + " search from base '" + searchBase + "'");
+
+            return tryOtherCredentialsProviders(name, password);
+        }
+
+        StringBuffer dnBuffer = new StringBuffer();
+
         try {
             if (log.isDebugEnabled()) {
-                log.debug("LDAPCredentialsProvider.authenticate");
+                if ((results != null) && results.hasMore()) {
+                    log.debug("Found users");
+                } else {
+                    log.debug("No users found");
+                }
             }
 
-            DirContext ctx = new InitialDirContext(env);
-            StringBuffer filterBuffer = new StringBuffer();
-            filterBuffer.append(uidSearchName);
-            filterBuffer.append("=");
-            filterBuffer.append(name);
-
-            String[] attrIDs = {uidSearchName};
-            SearchControls ctls = new SearchControls();
-            ctls.setReturningAttributes(attrIDs);
-            ctls.setSearchScope(SearchControls.SUBTREE_SCOPE);
-
-            NamingEnumeration results = ctx.search(searchBase, filterBuffer.toString(), ctls);
-
-            StringBuffer dnBuffer = new StringBuffer();
-
             while ((results != null) && results.hasMore()) {
                 SearchResult sr = (SearchResult) results.next();
                 dnBuffer.append(sr.getName());
                 dnBuffer.append(",");
                 dnBuffer.append(searchBase);
-                ctx.removeFromEnvironment(javax.naming.Context.SECURITY_PRINCIPAL);
-                ctx.removeFromEnvironment(javax.naming.Context.SECURITY_CREDENTIALS);
-                ctx.addToEnvironment(javax.naming.Context.SECURITY_PRINCIPAL, dnBuffer.toString());
-                ctx.addToEnvironment(javax.naming.Context.SECURITY_CREDENTIALS, password);
+
+                try {
+                    ctx.removeFromEnvironment(Context.SECURITY_PRINCIPAL);
+                    ctx.removeFromEnvironment(Context.SECURITY_CREDENTIALS);
+                    ctx.addToEnvironment(Context.SECURITY_PRINCIPAL, dnBuffer.toString());
+                    ctx.addToEnvironment(Context.SECURITY_CREDENTIALS, password);
+                } catch (NamingException e) {
+                    log.error("Connected and searched LDAP, but encountered unexpected error when switching authentication details.", e);
+
+                    return tryOtherCredentialsProviders(name, password);
+                }
+
                 ctls = new SearchControls();
                 ctls.setReturningAttributes(new String[0]);
                 ctls.setSearchScope(SearchControls.OBJECT_SCOPE);
 
-                NamingEnumeration authResults = ctx.search(dnBuffer.toString(), filterBuffer.toString(), ctls);
+                if (log.isDebugEnabled()) {
+                    log.debug("Searching below '" + dnBuffer + "' for '" + filterBuffer + "'");
+                }
+
+                try {
+                    ctx.search(dnBuffer.toString(), filterBuffer.toString(), ctls);
+                } catch (AuthenticationException ae) {
+                    if (log.isDebugEnabled()) {
+                        log.debug("User with dn '" + dnBuffer + "' found, but wrong password entered.");
+                    }
+
+                    return false;
+                } catch (NamingException e) {
+                    log.error("Initial connect and search successful, but second phase connection to LDAP as '" + dnBuffer + "' failed.", e);
+
+                    return tryOtherCredentialsProviders(name, password);
+                }
+
+                if (log.isDebugEnabled()) {
+                    log.debug("User '" + name + "' successfully authenticated; caching for " + timeout + " ms");
+                }
+
                 cache.put(name, new TimeAndPassword(System.currentTimeMillis() + timeout, password));
 
                 return true;
             }
-        } catch (NamingException ne) {
-            // The authentication against the LDAP has failed (the user DOES exist in LDAP)
-            // Do NOT try other providers
-            return false;
+        } catch (NamingException e) {
+            log.error("Connected but encountered error checking if LDAP had more results.", e);
         }
 
-        if (
-            // If we are here it means that the user was NOT found in the LDAP server,
-            // try other providers.
-            log.isDebugEnabled()) {
-            // If we are here it means that the user was NOT found in the LDAP server,
-            // try other providers.
-            log.debug("Couldn't find the user in LDAP server, trying other Credential Providers");
-        }
-
-        Collection credentialsProviders = UserManager.getInstance().getCredentialsProviders();
-
-        for (Iterator iterator = credentialsProviders.iterator();
-                iterator.hasNext();) {
-            CredentialsProvider provider = (CredentialsProvider) iterator.next();
-            boolean isLDAP = provider instanceof LDAPCredentialsProvider;
-
-            if (!isLDAP) {
-                if (provider.handles(name)) {
-                    log.debug("Seeing if " + provider.getClass().getName() + " authenticates()");
-
-                    boolean result = provider.authenticate(name, password);
-
-                    if (result) {
-                        cache.put(name, new TimeAndPassword(System.currentTimeMillis() + timeout, password));
-
-                        return true;
-                    } else {
-                        return false;
-                    }
-                }
-            }
-        }
-
-        return false;
+        return tryOtherCredentialsProviders(name, password);
     }
 
     public boolean changePassword(String name, String password) {
     }
 
     public boolean handles(String name) {
-        if (log.isDebugEnabled()) {
-            log.debug("LDAPCredentialsProvider.handles");
-        }
-
         // check cache
         TimeAndPassword tp = (TimeAndPassword) cache.get(name);
 
         if ((tp != null) && (tp.time > System.currentTimeMillis())) {
+            if (log.isDebugEnabled()) {
+                log.debug("Cached lookup: Credentials for '" + name + "' will be handled by LDAP provider");
+            }
+
             return true;
         }
 
 
             if (!isLDAP) {
                 if (provider.handles(name)) {
+                    if (log.isDebugEnabled()) {
+                        log.debug("'" + name + "' will be handled by LDAP");
+                    }
+
                     handles = true;
 
                     break;
             }
         }
 
+        if (log.isDebugEnabled()) {
+            if (handles == false) {
+                log.debug("Credentials for '" + name + "' NOT handled by LDAP, because '" + name + "' not handled by any other credentials provider. Check you have at least one other" + " credentials provider, and that they contain this user.");
+            }
+        }
+
         return handles;
     }
 
     public boolean init(Properties properties) {
         if (log.isDebugEnabled()) {
-            log.debug("LDAPCredentialsProvider.init");
+            log.debug("LDAPCredentialsProvider $Revision: 1.3 $ initializing");
         }
 
         env = new Hashtable(properties);
         return true;
     }
 
+    /** Loop through non-LDAP credentials providers and tries to authenticate against them. */
+    private boolean tryOtherCredentialsProviders(String name, String password) {
+        if (log.isDebugEnabled()) {
+            log.debug("Couldn't authenticate against LDAP server, trying other CredentialsProviders");
+        }
+
+        Collection credentialsProviders = UserManager.getInstance().getCredentialsProviders();
+
+        for (Iterator iterator = credentialsProviders.iterator();
+                iterator.hasNext();) {
+            CredentialsProvider provider = (CredentialsProvider) iterator.next();
+            boolean isLDAP = provider instanceof LDAPCredentialsProvider;
+
+            if (!isLDAP) {
+                if (provider.handles(name)) {
+                    if (log.isDebugEnabled()) {
+                        log.debug("Provider '" + provider.getClass().getName() + "' handles user; checking authentication...");
+                    }
+
+                    boolean result = provider.authenticate(name, password);
+
+                    if (result) {
+                        if (log.isDebugEnabled()) {
+                            log.debug("User authenticated by '" + provider.getClass().getName() + "'");
+                        }
+
+                        cache.put(name, new TimeAndPassword(System.currentTimeMillis() + timeout, password));
+
+                        return true;
+                    } else {
+                        if (log.isDebugEnabled()) {
+                            log.debug("Provider '" + provider.getClass().getName() + "' failed to authenticate user.");
+                        }
+
+                        return false;
+                    }
+                }
+            }
+        }
+
+        if (log.isDebugEnabled()) {
+            log.debug("No non-LDAP authenticators could authenticate this user");
+        }
+
+        return false;
+    }
+
     //~ Inner Classes //////////////////////////////////////////////////////////
 
     private class TimeAndPassword {