activemq-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jbono...@apache.org
Subject [activemq] branch activemq-5.15.x updated: AMQ-7137 - Implement abort() properly in the LoginModules. Also fix a bug in LdapLoginModule relating to Logout
Date Wed, 15 May 2019 06:17:08 GMT
This is an automated email from the ASF dual-hosted git repository.

jbonofre pushed a commit to branch activemq-5.15.x
in repository https://gitbox.apache.org/repos/asf/activemq.git


The following commit(s) were added to refs/heads/activemq-5.15.x by this push:
     new c0008f7  AMQ-7137 - Implement abort() properly in the LoginModules. Also fix a bug
in LdapLoginModule relating to Logout
c0008f7 is described below

commit c0008f710ae8d481d37164612b99f10f643df8bd
Author: Colm O hEigeartaigh <coheigea@apache.org>
AuthorDate: Wed Jan 16 14:10:02 2019 +0000

    AMQ-7137 - Implement abort() properly in the LoginModules. Also fix a bug in LdapLoginModule
relating to Logout
---
 .../activemq/jaas/CertificateLoginModule.java      | 43 +++++++++++----
 .../org/apache/activemq/jaas/GuestLoginModule.java | 38 ++++++++++----
 .../org/apache/activemq/jaas/LDAPLoginModule.java  | 42 +++++++++++++--
 .../activemq/jaas/PropertiesLoginModule.java       | 61 ++++++++++++++--------
 4 files changed, 136 insertions(+), 48 deletions(-)

diff --git a/activemq-jaas/src/main/java/org/apache/activemq/jaas/CertificateLoginModule.java
b/activemq-jaas/src/main/java/org/apache/activemq/jaas/CertificateLoginModule.java
index f2a6528..dd84018 100644
--- a/activemq-jaas/src/main/java/org/apache/activemq/jaas/CertificateLoginModule.java
+++ b/activemq-jaas/src/main/java/org/apache/activemq/jaas/CertificateLoginModule.java
@@ -49,10 +49,13 @@ public abstract class CertificateLoginModule extends PropertiesLoader
implements
     private CallbackHandler callbackHandler;
     private Subject subject;
 
-    private X509Certificate certificates[];
     private String username;
     private Set<Principal> principals = new HashSet<Principal>();
 
+    /** the authentication status*/
+    private boolean succeeded = false;
+    private boolean commitSucceeded = false;
+
     /**
      * Overriding to allow for proper initialization. Standard JAAS.
      */
@@ -78,7 +81,7 @@ public abstract class CertificateLoginModule extends PropertiesLoader implements
         } catch (UnsupportedCallbackException uce) {
             throw new LoginException(uce.getMessage() + " Unable to obtain client certificates.");
         }
-        certificates = ((CertificateCallback)callbacks[0]).getCertificates();
+        X509Certificate[] certificates = ((CertificateCallback)callbacks[0]).getCertificates();
 
         username = getUserNameForCertificates(certificates);
         if (username == null) {
@@ -88,6 +91,7 @@ public abstract class CertificateLoginModule extends PropertiesLoader implements
         if (debug) {
             LOG.debug("Certificate for user: " + username);
         }
+        succeeded = true;
         return true;
     }
 
@@ -96,6 +100,15 @@ public abstract class CertificateLoginModule extends PropertiesLoader
implements
      */
     @Override
     public boolean commit() throws LoginException {
+        if (debug) {
+            LOG.debug("commit");
+        }
+
+        if (!succeeded) {
+            clear();
+            return false;
+        }
+
         principals.add(new UserPrincipal(username));
 
         for (String group : getUserGroups(username)) {
@@ -104,11 +117,8 @@ public abstract class CertificateLoginModule extends PropertiesLoader
implements
 
         subject.getPrincipals().addAll(principals);
 
-        clear();
-
-        if (debug) {
-            LOG.debug("commit");
-        }
+        username = null;
+        commitSucceeded = true;
         return true;
     }
 
@@ -117,11 +127,19 @@ public abstract class CertificateLoginModule extends PropertiesLoader
implements
      */
     @Override
     public boolean abort() throws LoginException {
-        clear();
-
         if (debug) {
             LOG.debug("abort");
         }
+        if (!succeeded) {
+            return false;
+        } else if (succeeded && commitSucceeded) {
+            // we succeeded, but another required module failed
+            logout();
+        } else {
+            // our commit failed
+            clear();
+            succeeded = false;
+        }
         return true;
     }
 
@@ -131,11 +149,14 @@ public abstract class CertificateLoginModule extends PropertiesLoader
implements
     @Override
     public boolean logout() {
         subject.getPrincipals().removeAll(principals);
-        principals.clear();
+        clear();
 
         if (debug) {
             LOG.debug("logout");
         }
+
+        succeeded = false;
+        commitSucceeded = false;
         return true;
     }
 
@@ -143,8 +164,8 @@ public abstract class CertificateLoginModule extends PropertiesLoader
implements
      * Helper method.
      */
     private void clear() {
-        certificates = null;
         username = null;
+        principals.clear();
     }
 
     /**
diff --git a/activemq-jaas/src/main/java/org/apache/activemq/jaas/GuestLoginModule.java b/activemq-jaas/src/main/java/org/apache/activemq/jaas/GuestLoginModule.java
index 621083d..724e158 100644
--- a/activemq-jaas/src/main/java/org/apache/activemq/jaas/GuestLoginModule.java
+++ b/activemq-jaas/src/main/java/org/apache/activemq/jaas/GuestLoginModule.java
@@ -54,7 +54,10 @@ public class GuestLoginModule implements LoginModule {
     private boolean credentialsInvalidate;
     private Set<Principal> principals = new HashSet<Principal>();
     private CallbackHandler callbackHandler;
-    private boolean loginSucceeded;
+
+    /** the authentication status*/
+    private boolean succeeded = false;
+    private boolean commitSucceeded = false;
 
     @Override
     public void initialize(Subject subject, CallbackHandler callbackHandler, Map sharedState,
Map options) {
@@ -79,7 +82,7 @@ public class GuestLoginModule implements LoginModule {
 
     @Override
     public boolean login() throws LoginException {
-        loginSucceeded = true;
+        succeeded = true;
         if (credentialsInvalidate) {
             PasswordCallback passwordCallback = new PasswordCallback("Password: ", false);
             try {
@@ -88,7 +91,7 @@ public class GuestLoginModule implements LoginModule {
                      if (debug) {
                         LOG.debug("Guest login failing (credentialsInvalidate=true) on presence
of a password");
                      }
-                     loginSucceeded = false;
+                     succeeded = false;
                      passwordCallback.clearPassword();
                  };
              } catch (IOException ioe) {
@@ -96,21 +99,24 @@ public class GuestLoginModule implements LoginModule {
              }
         }
         if (debug) {
-            LOG.debug("Guest login " + loginSucceeded);
+            LOG.debug("Guest login " + succeeded);
         }
-        return loginSucceeded;
+        return succeeded;
     }
 
     @Override
     public boolean commit() throws LoginException {
-        if (loginSucceeded) {
-            subject.getPrincipals().addAll(principals);
-        }
-
         if (debug) {
             LOG.debug("commit");
         }
-        return loginSucceeded;
+
+        if (!succeeded) {
+            return false;
+        }
+
+        subject.getPrincipals().addAll(principals);
+        commitSucceeded = true;
+        return true;
     }
 
     @Override
@@ -119,6 +125,15 @@ public class GuestLoginModule implements LoginModule {
         if (debug) {
             LOG.debug("abort");
         }
+        if (!succeeded) {
+            return false;
+        } else if (succeeded && commitSucceeded) {
+            // we succeeded, but another required module failed
+            logout();
+        } else {
+            // our commit failed
+            succeeded = false;
+        }
         return true;
     }
 
@@ -129,6 +144,9 @@ public class GuestLoginModule implements LoginModule {
         if (debug) {
             LOG.debug("logout");
         }
+
+        succeeded = false;
+        commitSucceeded = false;
         return true;
     }
 }
diff --git a/activemq-jaas/src/main/java/org/apache/activemq/jaas/LDAPLoginModule.java b/activemq-jaas/src/main/java/org/apache/activemq/jaas/LDAPLoginModule.java
index f0834a0..dced7ce 100644
--- a/activemq-jaas/src/main/java/org/apache/activemq/jaas/LDAPLoginModule.java
+++ b/activemq-jaas/src/main/java/org/apache/activemq/jaas/LDAPLoginModule.java
@@ -72,9 +72,13 @@ public class LDAPLoginModule implements LoginModule {
     private Subject subject;
     private CallbackHandler handler;  
     private LDAPLoginProperty [] config;
-    private String username;
+    private Principal user;
     private Set<GroupPrincipal> groups = new HashSet<GroupPrincipal>();
 
+    /** the authentication status*/
+    private boolean succeeded = false;
+    private boolean commitSucceeded = false;
+
     @Override
     public void initialize(Subject subject, CallbackHandler callbackHandler, Map sharedState,
Map options) {
         this.subject = subject;
@@ -118,7 +122,7 @@ public class LDAPLoginModule implements LoginModule {
         
         String password;
         
-        username = ((NameCallback)callbacks[0]).getName();
+        String username = ((NameCallback)callbacks[0]).getName();
         if (username == null)
         	return false;
         	
@@ -130,28 +134,56 @@ public class LDAPLoginModule implements LoginModule {
         // authenticate will throw LoginException
         // in case of failed authentication
         authenticate(username, password);
+
+        user = new UserPrincipal(username);
+        succeeded = true;
         return true;
     }
 
     @Override
     public boolean logout() throws LoginException {
-        username = null;
+        subject.getPrincipals().remove(user);
+        subject.getPrincipals().removeAll(groups);
+
+        user = null;
+        groups.clear();
+
+        succeeded = false;
+        commitSucceeded = false;
         return true;
     }
 
     @Override
     public boolean commit() throws LoginException {
+        if (!succeeded) {
+            user = null;
+            groups.clear();
+            return false;
+        }
+
         Set<Principal> principals = subject.getPrincipals();
-        principals.add(new UserPrincipal(username));
+        principals.add(user);
         for (GroupPrincipal gp : groups) {
             principals.add(gp);
         }
+
+        commitSucceeded = true;
         return true;
     }
 
     @Override
     public boolean abort() throws LoginException {
-        username = null;
+        if (!succeeded) {
+            return false;
+        } else if (succeeded && commitSucceeded) {
+            // we succeeded, but another required module failed
+            logout();
+        } else {
+            // our commit failed
+            user = null;
+            groups.clear();
+            succeeded = false;
+        }
         return true;
     }
 
diff --git a/activemq-jaas/src/main/java/org/apache/activemq/jaas/PropertiesLoginModule.java
b/activemq-jaas/src/main/java/org/apache/activemq/jaas/PropertiesLoginModule.java
index 5346bd7..153a125 100644
--- a/activemq-jaas/src/main/java/org/apache/activemq/jaas/PropertiesLoginModule.java
+++ b/activemq-jaas/src/main/java/org/apache/activemq/jaas/PropertiesLoginModule.java
@@ -50,13 +50,16 @@ public class PropertiesLoginModule extends PropertiesLoader implements
LoginModu
     private Map<String,Set<String>> groups;
     private String user;
     private final Set<Principal> principals = new HashSet<Principal>();
-    private boolean loginSucceeded;
+
+    /** the authentication status*/
+    private boolean succeeded = false;
+    private boolean commitSucceeded = false;
 
     @Override
     public void initialize(Subject subject, CallbackHandler callbackHandler, Map sharedState,
Map options) {
         this.subject = subject;
         this.callbackHandler = callbackHandler;
-        loginSucceeded = false;
+        succeeded = false;
         init(options);
         users = load(USER_FILE_PROP_NAME, "user", options).getProps();
         groups = load(GROUP_FILE_PROP_NAME, "group", options).invertedPropertiesValuesMap();
@@ -91,63 +94,77 @@ public class PropertiesLoginModule extends PropertiesLoader implements
LoginModu
         if (!password.equals(new String(tmpPassword))) {
             throw new FailedLoginException("Password does not match");
         }
-        loginSucceeded = true;
+        succeeded = true;
 
         if (debug) {
             LOG.debug("login " + user);
         }
-        return loginSucceeded;
+        return succeeded;
     }
 
     @Override
     public boolean commit() throws LoginException {
-        boolean result = loginSucceeded;
-        if (result) {
-            principals.add(new UserPrincipal(user));
-
-            Set<String> matchedGroups = groups.get(user);
-            if (matchedGroups != null) {
-                for (String entry : matchedGroups) {
-                    principals.add(new GroupPrincipal(entry));
-                }
+        if (!succeeded) {
+            clear();
+            if (debug) {
+                LOG.debug("commit, result: false");
             }
+            return false;
+        }
 
-            subject.getPrincipals().addAll(principals);
+        principals.add(new UserPrincipal(user));
+
+        Set<String> matchedGroups = groups.get(user);
+        if (matchedGroups != null) {
+            for (String entry : matchedGroups) {
+                principals.add(new GroupPrincipal(entry));
+            }
         }
 
-        // will whack loginSucceeded
-        clear();
+        subject.getPrincipals().addAll(principals);
 
         if (debug) {
-            LOG.debug("commit, result: " + result);
+            LOG.debug("commit, result: true");
         }
-        return result;
+
+        commitSucceeded = true;
+        return true;
     }
 
     @Override
     public boolean abort() throws LoginException {
-        clear();
-
         if (debug) {
             LOG.debug("abort");
         }
+        if (!succeeded) {
+            return false;
+        } else if (succeeded && commitSucceeded) {
+            // we succeeded, but another required module failed
+            logout();
+        } else {
+            // our commit failed
+            clear();
+            succeeded = false;
+        }
         return true;
     }
 
     @Override
     public boolean logout() throws LoginException {
         subject.getPrincipals().removeAll(principals);
-        principals.clear();
         clear();
         if (debug) {
             LOG.debug("logout");
         }
+
+        succeeded = false;
+        commitSucceeded = false;
         return true;
     }
 
     private void clear() {
         user = null;
-        loginSucceeded = false;
+        principals.clear();
     }
 
 }


Mime
View raw message