You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2016/06/15 20:26:17 UTC

svn commit: r1748629 - in /tomcat/trunk: java/org/apache/catalina/realm/LockOutRealm.java webapps/docs/changelog.xml webapps/docs/config/realm.xml

Author: markt
Date: Wed Jun 15 20:26:17 2016
New Revision: 1748629

URL: http://svn.apache.org/viewvc?rev=1748629&view=rev
Log:
Modify the lock out logic. Valid authentication attempts during the lock out period will no longer reset the lock out timer to zero. 

Modified:
    tomcat/trunk/java/org/apache/catalina/realm/LockOutRealm.java
    tomcat/trunk/webapps/docs/changelog.xml
    tomcat/trunk/webapps/docs/config/realm.xml

Modified: tomcat/trunk/java/org/apache/catalina/realm/LockOutRealm.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/realm/LockOutRealm.java?rev=1748629&r1=1748628&r2=1748629&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/realm/LockOutRealm.java (original)
+++ tomcat/trunk/java/org/apache/catalina/realm/LockOutRealm.java Wed Jun 15 20:26:17 2016
@@ -139,23 +139,9 @@ public class LockOutRealm extends Combin
             String nonce, String nc, String cnonce, String qop,
             String realmName, String md5a2) {
 
-        if (isLocked(username)) {
-            // Trying to authenticate a locked user is an automatic failure
-            registerAuthFailure(username);
-
-            log.warn(sm.getString("lockOutRealm.authLockedUser", username));
-            return null;
-        }
-
-        Principal authenticatedUser = super.authenticate(username, clientDigest,
-                nonce, nc, cnonce, qop, realmName, md5a2);
-
-        if (authenticatedUser == null) {
-            registerAuthFailure(username);
-        } else {
-            registerAuthSuccess(username);
-        }
-        return authenticatedUser;
+        Principal authenticatedUser = super.authenticate(username, clientDigest, nonce, nc, cnonce,
+                qop, realmName, md5a2);
+        return filterLockedAccounts(username, authenticatedUser);
     }
 
 
@@ -169,22 +155,8 @@ public class LockOutRealm extends Combin
      */
     @Override
     public Principal authenticate(String username, String credentials) {
-        if (isLocked(username)) {
-            // Trying to authenticate a locked user is an automatic failure
-            registerAuthFailure(username);
-
-            log.warn(sm.getString("lockOutRealm.authLockedUser", username));
-            return null;
-        }
-
         Principal authenticatedUser = super.authenticate(username, credentials);
-
-        if (authenticatedUser == null) {
-            registerAuthFailure(username);
-        } else {
-            registerAuthSuccess(username);
-        }
-        return authenticatedUser;
+        return filterLockedAccounts(username, authenticatedUser);
     }
 
 
@@ -202,22 +174,8 @@ public class LockOutRealm extends Combin
             username = certs[0].getSubjectDN().getName();
         }
 
-        if (isLocked(username)) {
-            // Trying to authenticate a locked user is an automatic failure
-            registerAuthFailure(username);
-
-            log.warn(sm.getString("lockOutRealm.authLockedUser", username));
-            return null;
-        }
-
         Principal authenticatedUser = super.authenticate(certs);
-
-        if (authenticatedUser == null) {
-            registerAuthFailure(username);
-        } else {
-            registerAuthSuccess(username);
-        }
-        return authenticatedUser;
+        return filterLockedAccounts(username, authenticatedUser);
     }
 
 
@@ -238,23 +196,9 @@ public class LockOutRealm extends Combin
 
             username = name.toString();
 
-            if (isLocked(username)) {
-                // Trying to authenticate a locked user is an automatic failure
-                registerAuthFailure(username);
+            Principal authenticatedUser = super.authenticate(gssContext, storeCreds);
 
-                log.warn(sm.getString("lockOutRealm.authLockedUser", username));
-                return null;
-            }
-
-            Principal authenticatedUser =
-                    super.authenticate(gssContext, storeCreds);
-
-            if (authenticatedUser == null) {
-                registerAuthFailure(username);
-            } else {
-                registerAuthSuccess(username);
-            }
-            return authenticatedUser;
+            return filterLockedAccounts(username, authenticatedUser);
         }
 
         // Fail in all other cases
@@ -262,6 +206,30 @@ public class LockOutRealm extends Combin
     }
 
 
+    /*
+     * Filters authenticated principals to ensure that <code>null</code> is
+     * returned for any user that is currently locked out.
+     */
+    private Principal filterLockedAccounts(String username, Principal authenticatedUser) {
+        // Register all failed authentications
+        if (authenticatedUser == null) {
+            registerAuthFailure(username);
+        }
+
+        if (isLocked(username)) {
+            // If the user is currently locked, authentication will always fail
+            log.warn(sm.getString("lockOutRealm.authLockedUser", username));
+            return null;
+        }
+
+        if (authenticatedUser != null) {
+            registerAuthSuccess(username);
+        }
+
+        return authenticatedUser;
+    }
+
+
     /**
      * Unlock the specified username. This will remove all records of
      * authentication failures for this user.

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1748629&r1=1748628&r2=1748629&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Wed Jun 15 20:26:17 2016
@@ -57,6 +57,11 @@
         ship with Tomcat that allows the HTTP status code used for HTTP -> HTTPS
         redirects to be controlled per Realm. (markt)
       </add>
+      <fix>
+        <bug>59708</bug>: Modify the LockOutRealm logic. Valid authentication
+        attempts during the lock out period will no longer reset the lock out
+        timer to zero. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">

Modified: tomcat/trunk/webapps/docs/config/realm.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/config/realm.xml?rev=1748629&r1=1748628&r2=1748629&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/config/realm.xml (original)
+++ tomcat/trunk/webapps/docs/config/realm.xml Wed Jun 15 20:26:17 2016
@@ -1034,7 +1034,11 @@
 
       <attribute name="lockOutTime" required="false">
        <p>The time (in seconds) a user is locked out for after too many
-       authentication failures. Defaults to 300 (5 minutes).</p>
+       authentication failures. Defaults to 300 (5 minutes). Further
+       authentication failures during the lock out time will cause the lock out
+       timer to reset to zero, effectively extending the lock out time. Valid
+       authentication attempts during the lock out period will not succeed but
+       will also not reset the lock out time.</p>
       </attribute>
 
     </attributes>



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org