Commits

Anonymous committed e925110

Patches from USER-76 and 74

  • Participants
  • Parent commits 9e78d8c

Comments (0)

Files changed (1)

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

      <property name="java.naming.security.principal">cn=admin,dc=example,dc=com</property>
      <property name="java.naming.security.credentials">secret</property>
      <property name="exclusive-access">true</property>
+     <property name="providerName">Intranet LDAP server</property>
     </provider>
   * </pre>
  * The security principal and credentials lines are optional, depending on whether your initial connection need be authenticated or not.
+ * The providerName property is also optional, and uniquely identifies this provider in debug logs when more than one is configured (providing fallback).
  * <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).
 
     //~ Instance fields ////////////////////////////////////////////////////////
 
-    private Hashtable env;
-    private String searchBase;
-    private String uidSearchName;
-    private long timeout;
+    // package-protected - only used in equals()
+    Hashtable env;
+    String providerName;
+    String searchBase;
+    String uidSearchName;
+    long timeout;
 
     //~ Methods ////////////////////////////////////////////////////////////////
 
 
         if ((tp != null) && tp.password.equals(password) && (tp.time > System.currentTimeMillis())) {
             if (log.isDebugEnabled()) {
-                log.debug("Successful authentication for " + name + " from cached LDAP lookup");
+                log.debug("Successful authentication for " + name + " from cached " + providerName() + " lookup");
             }
 
             return true;
         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);
+            log.error("Could not connect to " + providerName() + ". Please check your " + "host ('" + env.get(Context.PROVIDER_URL) + "'), " + "bind DN ('" + env.get(Context.SECURITY_PRINCIPAL) + "') and bind password.", e);
 
-            return tryOtherCredentialsProviders(name, password);
+            return tryNextCredentialsProviders(name, password);
         }
 
         StringBuffer filterBuffer = new StringBuffer(uidSearchName).append("=").append(name);
         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 + "'");
+            log.error("Connected to " + providerName() + ", but could not perform " + (env.containsKey(Context.SECURITY_PRINCIPAL) ? "authenticated" : "anonymous") + " search from base '" + searchBase + "'");
 
-            return tryOtherCredentialsProviders(name, password);
+            return tryNextCredentialsProviders(name, password);
         }
 
         StringBuffer dnBuffer;
         try {
             if (log.isDebugEnabled()) {
                 if ((results != null) && results.hasMore()) {
-                    log.debug("Found users");
+                    log.debug("Found user(s)");
                 } else {
                     log.debug("No users found");
                 }
                     ctx2.addToEnvironment(Context.SECURITY_PRINCIPAL, dnBuffer.toString());
                     ctx2.addToEnvironment(Context.SECURITY_CREDENTIALS, password);
                 } catch (NamingException e) {
-                    log.error("Connected and searched LDAP, but encountered unexpected error when switching authentication details.", e);
+                    log.error("Connected and searched " + providerName() + ", but encountered unexpected error when switching authentication details.", e);
 
                     continue;
                 }
                         log.debug("User with dn '" + dnBuffer + "' found, but authentication failed.");
                     }
 
-                    continue;
+                    // If user exists in LDAP but password was incorrect, DO NOT fall back - fail immediately
+                    return false;
                 } catch (NamingException e) {
-                    log.error("Initial connect and search successful, but second phase connection to LDAP as '" + dnBuffer + "' failed.", e);
+                    log.error("Initial connect and search successful, but second phase connection to " + providerName() + " as '" + dnBuffer + "' failed.", e);
 
                     continue;
                 }
                 return true;
             }
         } catch (PartialResultException pre) {
-            log.error("Connected but encountered error checking if LDAP had more results.  For Unprocessed Continuation References, try adding <property name=\"java.naming.referral\">follow</property> to the LDAPCredentialsProvider config.", pre);
+            log.error("Connected but encountered error checking if " + providerName() + " had more results.  For Unprocessed Continuation References, try adding <property name=\"java.naming.referral\">follow</property> to the LDAPCredentialsProvider config.", pre);
         } catch (NamingException e) {
-            log.error("Connected but encountered error checking if LDAP had more results.", e);
+            log.error("Connected but encountered error checking if " + providerName() + " had more results.", e);
         }
 
-        return tryOtherCredentialsProviders(name, password);
+        return tryNextCredentialsProviders(name, password);
     }
 
     public boolean changePassword(String name, String password) {
         return false;
     }
 
+    public boolean equals(Object o) {
+        if ((o != null) && o.getClass().equals(this.getClass())) {
+            LDAPCredentialsProvider other = (LDAPCredentialsProvider) o;
+
+            return env.equals(((LDAPCredentialsProvider) other).env);
+        }
+
+        return false;
+    }
+
     public void flushCaches() {
         // flush the TimeAndPassword cache
         cache = new HashMap();
 
         if ((tp != null) && (tp.time > System.currentTimeMillis())) {
             if (log.isDebugEnabled()) {
-                log.debug("Cached lookup: Credentials for '" + name + "' will be handled by LDAP provider");
+                log.debug("Cached lookup: Credentials for '" + name + "' could be handled by " + providerName());
             }
 
             return true;
             if (!isLDAP) {
                 if (provider.handles(name)) {
                     if (log.isDebugEnabled()) {
-                        log.debug("'" + name + "' will be handled by LDAP");
+                        log.debug("'" + name + "' could be handled by " + providerName());
                     }
 
                     handles = true;
         return handles;
     }
 
+    public int hashCode() {
+        return env.hashCode();
+    }
+
     public boolean init(Properties properties) {
         if (log.isDebugEnabled()) {
-            log.debug("LDAPCredentialsProvider $Revision: 1.6 $ initializing");
+            log.debug("Credentials Provider " + providerName() + " $Revision: 1.7 $ initializing");
         }
 
         env = new Hashtable(properties);
         env.put(javax.naming.Context.SECURITY_AUTHENTICATION, "simple");
         searchBase = properties.getProperty("searchBase");
         uidSearchName = properties.getProperty("uidSearchName");
+        providerName = properties.getProperty("providerName");
 
         try {
             timeout = Long.parseLong(properties.getProperty("cacheTimeout"));
         return true;
     }
 
-    /** Loop through non-LDAP credentials providers and tries to authenticate against them. */
-    private boolean tryOtherCredentialsProviders(String name, String password) {
+    private final String providerName() {
+        return (providerName == null) ? "LDAP" : ("LDAP provider '" + providerName + "'");
+    }
+
+    /** Loop through CredentialsProviders after ours, and pass off to the first one which handles this user. */
+    private boolean tryNextCredentialsProviders(String name, String password) {
         if (log.isDebugEnabled()) {
-            log.debug("Couldn't authenticate against LDAP server, trying other CredentialsProviders");
+            log.debug("Couldn't authenticate against " + providerName() + ", trying other CredentialsProviders");
         }
 
         Collection credentialsProviders = UserManager.getInstance().getCredentialsProviders();
 
+        boolean onUntriedProvider = false;
+
         for (Iterator iterator = credentialsProviders.iterator();
                 iterator.hasNext();) {
-            CredentialsProvider provider = (CredentialsProvider) iterator.next();
-            boolean isLDAP = provider instanceof LDAPCredentialsProvider;
+            CredentialsProvider nextProvider = (CredentialsProvider) iterator.next();
 
-            if (!isLDAP) {
-                if (provider.handles(name)) {
+            if (!onUntriedProvider) { // Skip providers we've seen before (including this one)
+
+                if (this.equals(nextProvider)) {
+                    onUntriedProvider = true;
+                }
+
+                continue;
+            }
+
+            String nextProviderName = null;
+
+            if (log.isDebugEnabled()) {
+                nextProviderName = ((nextProvider instanceof LDAPCredentialsProvider) ? ((LDAPCredentialsProvider) nextProvider).providerName() : nextProvider.getClass().getName());
+            }
+
+            if (nextProvider.handles(name)) {
+                if (log.isDebugEnabled()) {
+                    log.debug("Next provider " + nextProviderName + "' could handle user; checking authentication...");
+                }
+
+                boolean result = nextProvider.authenticate(name, password);
+
+                if (result) {
                     if (log.isDebugEnabled()) {
-                        log.debug("Provider '" + provider.getClass().getName() + "' handles user; checking authentication...");
+                        log.debug("User authenticated by '" + nextProviderName + "'");
                     }
 
-                    boolean result = provider.authenticate(name, password);
+                    cache.put(name, new TimeAndPassword(System.currentTimeMillis() + timeout, password));
 
-                    if (result) {
-                        if (log.isDebugEnabled()) {
-                            log.debug("User authenticated by '" + provider.getClass().getName() + "'");
-                        }
+                    return true;
+                } else {
+                    if (log.isDebugEnabled()) {
+                        log.debug("Next provider '" + nextProviderName + "' failed to authenticate user.");
+                    }
 
-                        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;
-                    }
+                    return false;
                 }
             }
         }