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 2019/04/30 10:09:56 UTC

[tomcat] branch 8.5.x updated: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63333

This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/8.5.x by this push:
     new f83d0e1  Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63333
f83d0e1 is described below

commit f83d0e16c16f2bb42a42faf6f387ef114e0693e8
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Apr 30 11:03:45 2019 +0100

    Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63333
    
    Override the isAvailable() method in the JAASRealm so that only login
    failures caused by invalid credentials trigger account lock out when the
    LockOutRealm is in use.
    
    Patch provided by jchobantonov.
---
 java/org/apache/catalina/realm/JAASRealm.java | 50 +++++++++++++++++++++++----
 webapps/docs/changelog.xml                    |  6 ++++
 2 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/java/org/apache/catalina/realm/JAASRealm.java b/java/org/apache/catalina/realm/JAASRealm.java
index 469e80d..09216a4 100644
--- a/java/org/apache/catalina/realm/JAASRealm.java
+++ b/java/org/apache/catalina/realm/JAASRealm.java
@@ -175,6 +175,14 @@ public class JAASRealm extends RealmBase {
     protected Configuration jaasConfiguration;
     protected volatile boolean jaasConfigurationLoaded = false;
 
+    /**
+     * Keeps track if JAAS invocation of login modules was successful or not. By
+     * default it is true unless we detect JAAS login module can't perform the
+     * login. This will be used for realm's {@link #isAvailable()} status so
+     * that {@link LockOutRealm} will not lock the user out if JAAS login
+     * modules are unavailable to perform the actual login.
+     */
+    private volatile boolean invocationSuccess = true;
 
     // ------------------------------------------------------------- Properties
 
@@ -402,7 +410,10 @@ public class JAASRealm extends RealmBase {
         } catch (Throwable e) {
             ExceptionUtils.handleThrowable(e);
             log.error(sm.getString("jaasRealm.unexpectedError"), e);
-            return (null);
+            // There is configuration issue with JAAS so mark the realm as
+            // unavailable
+            invocationSuccess = false;
+            return null;
         } finally {
             if(!isUseContextClassLoader()) {
               Thread.currentThread().setContextClassLoader(ocl);
@@ -417,6 +428,11 @@ public class JAASRealm extends RealmBase {
         try {
             loginContext.login();
             subject = loginContext.getSubject();
+            // We were able to perform login successfully so mark JAAS realm as
+            // available as it could have been set to false in prior attempts.
+            // Change invocationSuccess variable only when we know the outcome
+            // of the JAAS operation to keep variable consistent.
+            invocationSuccess = true;
             if (subject == null) {
                 if( log.isDebugEnabled())
                     log.debug(sm.getString("jaasRealm.failedLogin", username));
@@ -425,22 +441,37 @@ public class JAASRealm extends RealmBase {
         } catch (AccountExpiredException e) {
             if (log.isDebugEnabled())
                 log.debug(sm.getString("jaasRealm.accountExpired", username));
-            return (null);
+            // JAAS checked LoginExceptions are successful authentication
+            // invocations so mark JAAS realm as available
+            invocationSuccess = true;
+            return null;
         } catch (CredentialExpiredException e) {
             if (log.isDebugEnabled())
                 log.debug(sm.getString("jaasRealm.credentialExpired", username));
-            return (null);
+            // JAAS checked LoginExceptions are successful authentication
+            // invocations so mark JAAS realm as available
+            invocationSuccess = true;
+            return null;
         } catch (FailedLoginException e) {
             if (log.isDebugEnabled())
                 log.debug(sm.getString("jaasRealm.failedLogin", username));
-            return (null);
+            // JAAS checked LoginExceptions are successful authentication
+            // invocations so mark JAAS realm as available
+            invocationSuccess = true;
+            return null;
         } catch (LoginException e) {
             log.warn(sm.getString("jaasRealm.loginException", username), e);
-            return (null);
+            // JAAS checked LoginExceptions are successful authentication
+            // invocations so mark JAAS realm as available
+            invocationSuccess = true;
+            return null;
         } catch (Throwable e) {
             ExceptionUtils.handleThrowable(e);
             log.error(sm.getString("jaasRealm.unexpectedError"), e);
-            return (null);
+            // JAAS throws exception different than LoginException so mark the
+            // realm as unavailable
+            invocationSuccess = false;
+            return null;
         }
 
         if( log.isDebugEnabled())
@@ -459,6 +490,8 @@ public class JAASRealm extends RealmBase {
         return (principal);
         } catch( Throwable t) {
             log.error( "error ", t);
+            //JAAS throws exception different than LoginException so mark the realm as unavailable
+            invocationSuccess = false;
             return null;
         }
     }
@@ -659,4 +692,9 @@ public class JAASRealm extends RealmBase {
         }
 
     }
+
+    @Override
+    public boolean isAvailable() {
+        return invocationSuccess;
+    }
 }
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index deeedb4..6591d46 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -89,6 +89,12 @@
         defined in <code>server.xml</code> with a <code>docBase</code> but not
         the optional <code>path</code>. (markt)
       </fix>
+      <fix>
+        <bug>63333</bug>: Override the <code>isAvailable()</code> method in the
+        <code>JAASRealm</code> so that only login failures caused by invalid
+        credentials trigger account lock out when the <code>LockOutRealm</code>
+        is in use. Patch provided by jchobantonov. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">


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