From 610b87aed8ac0c98a97af5637750b84fe0b15e4a Mon Sep 17 00:00:00 2001 From: Dmytro Chebotarskyi Date: Thu, 24 Jun 2021 13:10:06 -0700 Subject: [PATCH] AMQ-6148 re-using LDAP context for authentication --- .../apache/activemq/jaas/LDAPLoginModule.java | 96 +++++++++---------- 1 file changed, 44 insertions(+), 52 deletions(-) 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 e1dc703bed..41edef9cdf 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 @@ -70,10 +70,10 @@ public class LDAPLoginModule implements LoginModule { protected DirContext context; private Subject subject; - private CallbackHandler handler; + private CallbackHandler handler; private LDAPLoginProperty [] config; private Principal user; - private Set groups = new HashSet(); + private Set groups = new HashSet<>(); /** the authentication status*/ private boolean succeeded = false; @@ -83,7 +83,7 @@ public class LDAPLoginModule implements LoginModule { public void initialize(Subject subject, CallbackHandler callbackHandler, Map sharedState, Map options) { this.subject = subject; this.handler = callbackHandler; - + config = new LDAPLoginProperty [] { new LDAPLoginProperty (INITIAL_CONTEXT_FACTORY, (String)options.get(INITIAL_CONTEXT_FACTORY)), new LDAPLoginProperty (CONNECTION_URL, (String)options.get(CONNECTION_URL)), @@ -114,18 +114,16 @@ public class LDAPLoginModule implements LoginModule { callbacks[1] = new PasswordCallback("Password", false); try { handler.handle(callbacks); - } catch (IOException ioe) { + } catch (IOException | UnsupportedCallbackException ioe) { throw (LoginException)new LoginException().initCause(ioe); - } catch (UnsupportedCallbackException uce) { - throw (LoginException)new LoginException().initCause(uce); } - + String password; - + String username = ((NameCallback)callbacks[0]).getName(); if (username == null) return false; - + if (((PasswordCallback)callbacks[1]).getPassword() != null) password = new String(((PasswordCallback)callbacks[1]).getPassword()); else @@ -163,9 +161,7 @@ public class LDAPLoginModule implements LoginModule { Set principals = subject.getPrincipals(); principals.add(user); - for (GroupPrincipal gp : groups) { - principals.add(gp); - } + principals.addAll(groups); commitSucceeded = true; return true; @@ -175,7 +171,7 @@ public class LDAPLoginModule implements LoginModule { public boolean abort() throws LoginException { if (!succeeded) { return false; - } else if (succeeded && commitSucceeded) { + } else if (commitSucceeded) { // we succeeded, but another required module failed logout(); } else { @@ -187,11 +183,16 @@ public class LDAPLoginModule implements LoginModule { return true; } - protected void close(DirContext context) { + protected void closeContext() { + if (context == null) { + return; + } try { context.close(); } catch (Exception e) { log.error(e.toString()); + } finally { + context = null; } } @@ -199,25 +200,23 @@ public class LDAPLoginModule implements LoginModule { MessageFormat userSearchMatchingFormat; boolean userSearchSubtreeBool; - - DirContext context = null; if (log.isDebugEnabled()) { log.debug("Create the LDAP initial context."); } try { - context = open(); + openContext(); } catch (NamingException ne) { FailedLoginException ex = new FailedLoginException("Error opening LDAP connection"); ex.initCause(ne); throw ex; } - + if (!isLoginPropertySet(USER_SEARCH_MATCHING)) return false; userSearchMatchingFormat = new MessageFormat(getLDAPPropertyValue(USER_SEARCH_MATCHING)); - userSearchSubtreeBool = Boolean.valueOf(getLDAPPropertyValue(USER_SEARCH_SUBTREE)).booleanValue(); + userSearchSubtreeBool = Boolean.valueOf(getLDAPPropertyValue(USER_SEARCH_SUBTREE)); try { @@ -232,7 +231,7 @@ public class LDAPLoginModule implements LoginModule { } // setup attributes - List list = new ArrayList(); + List list = new ArrayList<>(); if (isLoginPropertySet(USER_ROLE_NAME)) { list.add(getLDAPPropertyValue(USER_ROLE_NAME)); } @@ -284,9 +283,7 @@ public class LDAPLoginModule implements LoginModule { dn = path; } } catch (URISyntaxException e) { - if (context != null) { - close(context); - } + closeContext(); FailedLoginException ex = new FailedLoginException("Error parsing absolute name as URI."); ex.initCause(e); throw ex; @@ -324,18 +321,12 @@ public class LDAPLoginModule implements LoginModule { ex.initCause(e); throw ex; } catch (NamingException e) { - if (context != null) { - close(context); - } FailedLoginException ex = new FailedLoginException("Error contacting LDAP"); ex.initCause(e); throw ex; + } finally { + closeContext(); } - - if (context != null) { - close(context); - } - return true; } @@ -345,11 +336,11 @@ public class LDAPLoginModule implements LoginModule { boolean roleSearchSubtreeBool; boolean expandRolesBool; roleSearchMatchingFormat = new MessageFormat(getLDAPPropertyValue(ROLE_SEARCH_MATCHING)); - roleSearchSubtreeBool = Boolean.valueOf(getLDAPPropertyValue(ROLE_SEARCH_SUBTREE)).booleanValue(); - expandRolesBool = Boolean.valueOf(getLDAPPropertyValue(EXPAND_ROLES)).booleanValue(); - + roleSearchSubtreeBool = Boolean.parseBoolean(getLDAPPropertyValue(ROLE_SEARCH_SUBTREE)); + expandRolesBool = Boolean.parseBoolean(getLDAPPropertyValue(EXPAND_ROLES)); + if (list == null) { - list = new ArrayList(); + list = new ArrayList<>(); } if (!isLoginPropertySet(ROLE_NAME)) { return list; @@ -370,8 +361,8 @@ public class LDAPLoginModule implements LoginModule { log.debug(" base DN: " + getLDAPPropertyValue(ROLE_BASE)); log.debug(" filter: " + filter); } - HashSet haveSeenNames = new HashSet(); - Queue pendingNameExpansion = new LinkedList(); + HashSet haveSeenNames = new HashSet<>(); + Queue pendingNameExpansion = new LinkedList<>(); NamingEnumeration results = context.search(getLDAPPropertyValue(ROLE_BASE), filter, constraints); while (results.hasMore()) { SearchResult result = results.next(); @@ -436,7 +427,6 @@ public class LDAPLoginModule implements LoginModule { protected boolean bindUser(DirContext context, String dn, String password) throws NamingException { boolean isValid = false; - if (log.isDebugEnabled()) { log.debug("Binding the user."); } @@ -450,7 +440,6 @@ public class LDAPLoginModule implements LoginModule { log.debug("User " + dn + " successfully bound."); } } catch (AuthenticationException e) { - isValid = false; if (log.isDebugEnabled()) { log.debug("Authentication failed for dn=" + dn); } @@ -476,7 +465,7 @@ public class LDAPLoginModule implements LoginModule { return values; } if (values == null) { - values = new ArrayList(); + values = new ArrayList<>(); } Attribute attr = attrs.get(attrId); if (attr == null) { @@ -490,9 +479,12 @@ public class LDAPLoginModule implements LoginModule { return values; } - protected DirContext open() throws NamingException { + protected void openContext() throws NamingException { + if (context != null) { + return; + } try { - Hashtable env = new Hashtable(); + Hashtable env = new Hashtable<>(); env.put(Context.INITIAL_CONTEXT_FACTORY, getLDAPPropertyValue(INITIAL_CONTEXT_FACTORY)); if (isLoginPropertySet(CONNECTION_USERNAME)) { env.put(Context.SECURITY_PRINCIPAL, getLDAPPropertyValue(CONNECTION_USERNAME)); @@ -511,24 +503,24 @@ public class LDAPLoginModule implements LoginModule { context = new InitialDirContext(env); } catch (NamingException e) { + closeContext(); log.error(e.toString()); throw e; } - return context; } - + private String getLDAPPropertyValue (String propertyName){ - for (int i=0; i < config.length; i++ ) - if (config[i].getPropertyName() == propertyName) - return config[i].getPropertyValue(); + for (LDAPLoginProperty ldapLoginProperty : config) + if (ldapLoginProperty.getPropertyName().equals(propertyName)) + return ldapLoginProperty.getPropertyValue(); return null; } - + private boolean isLoginPropertySet(String propertyName) { - for (int i=0; i < config.length; i++ ) { - if (config[i].getPropertyName() == propertyName && (config[i].getPropertyValue() != null && !"".equals(config[i].getPropertyValue()))) - return true; - } + for (LDAPLoginProperty ldapLoginProperty : config) { + if (ldapLoginProperty.getPropertyName().equals(propertyName) && (ldapLoginProperty.getPropertyValue() != null && !"".equals(ldapLoginProperty.getPropertyValue()))) + return true; + } return false; }