diff --git a/src/main/java/org/elasticsearch/shield/authc/active_directory/ActiveDirectoryConnection.java b/src/main/java/org/elasticsearch/shield/authc/active_directory/ActiveDirectoryConnection.java index 88f00040ba8..49b548bfac0 100644 --- a/src/main/java/org/elasticsearch/shield/authc/active_directory/ActiveDirectoryConnection.java +++ b/src/main/java/org/elasticsearch/shield/authc/active_directory/ActiveDirectoryConnection.java @@ -9,7 +9,9 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.collect.ImmutableList; import org.elasticsearch.common.logging.ESLogger; import org.elasticsearch.common.logging.Loggers; +import org.elasticsearch.shield.authc.ldap.LdapException; import org.elasticsearch.shield.authc.support.ldap.AbstractLdapConnection; +import org.elasticsearch.shield.authc.support.ldap.ClosableNamingEnumeration; import javax.naming.NamingEnumeration; import javax.naming.NamingException; @@ -64,16 +66,15 @@ public class ActiveDirectoryConnection extends AbstractLdapConnection { groupsSearchCtls.setTimeLimit(timeoutMilliseconds); ImmutableList.Builder groups = ImmutableList.builder(); - try { - //Search for objects using the filter - NamingEnumeration groupsAnswer = jndiContext.search(groupSearchDN, groupsSearchFilter, groupsSearchCtls); + try (ClosableNamingEnumeration groupsAnswer = new ClosableNamingEnumeration( + jndiContext.search(groupSearchDN, groupsSearchFilter, groupsSearchCtls))) { //Loop through the search results while (groupsAnswer.hasMoreElements()) { SearchResult sr = (SearchResult) groupsAnswer.next(); groups.add(sr.getNameInNamespace()); } - } catch (NamingException ne) { + } catch (NamingException | LdapException ne) { throw new ActiveDirectoryException("Exception occurred fetching AD groups", bindDn, ne); } List groupList = groups.build(); @@ -93,29 +94,33 @@ public class ActiveDirectoryConnection extends AbstractLdapConnection { String userSearchFilter = "(objectClass=user)"; String userReturnedAtts[] = { "tokenGroups" }; userSearchCtls.setReturningAttributes(userReturnedAtts); - NamingEnumeration userAnswer = jndiContext.search(authenticatedUserDn(), userSearchFilter, userSearchCtls); + try (ClosableNamingEnumeration userAnswer = new ClosableNamingEnumeration( + jndiContext.search(authenticatedUserDn(), userSearchFilter, userSearchCtls))) { - //Loop through the search results - while (userAnswer.hasMoreElements()) { + //Loop through the search results + while (userAnswer.hasMoreElements()) { - SearchResult sr = (SearchResult) userAnswer.next(); - Attributes attrs = sr.getAttributes(); + SearchResult sr = (SearchResult) userAnswer.next(); + Attributes attrs = sr.getAttributes(); - if (attrs != null) { - for (NamingEnumeration ae = attrs.getAll(); ae.hasMore(); ) { - Attribute attr = (Attribute) ae.next(); - for (NamingEnumeration e = attr.getAll(); e.hasMore(); ) { - byte[] sid = (byte[]) e.next(); - groupsSearchFilter.append("(objectSid="); - groupsSearchFilter.append(binarySidToStringSid(sid)); - groupsSearchFilter.append(")"); + if (attrs != null) { + try (ClosableNamingEnumeration ae = new ClosableNamingEnumeration<>(attrs.getAll())) { + while (ae.hasMore() ) { + Attribute attr = (Attribute) ae.next(); + for (NamingEnumeration e = attr.getAll(); e.hasMore(); ) { + byte[] sid = (byte[]) e.next(); + groupsSearchFilter.append("(objectSid="); + groupsSearchFilter.append(binarySidToStringSid(sid)); + groupsSearchFilter.append(")"); + } + groupsSearchFilter.append(")"); + } } - groupsSearchFilter.append(")"); } } } - } catch (NamingException ne) { + } catch (NamingException | LdapException ne) { throw new ActiveDirectoryException("Exception occurred fetching AD groups", bindDn, ne); } return groupsSearchFilter.toString(); diff --git a/src/main/java/org/elasticsearch/shield/authc/active_directory/ActiveDirectoryConnectionFactory.java b/src/main/java/org/elasticsearch/shield/authc/active_directory/ActiveDirectoryConnectionFactory.java index 60f72fcc9d6..94cac7d8e07 100644 --- a/src/main/java/org/elasticsearch/shield/authc/active_directory/ActiveDirectoryConnectionFactory.java +++ b/src/main/java/org/elasticsearch/shield/authc/active_directory/ActiveDirectoryConnectionFactory.java @@ -10,12 +10,12 @@ import org.elasticsearch.common.collect.ImmutableMap; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.shield.ShieldSettingsException; +import org.elasticsearch.shield.authc.ldap.LdapException; import org.elasticsearch.shield.authc.support.SecuredString; +import org.elasticsearch.shield.authc.support.ldap.ClosableNamingEnumeration; import org.elasticsearch.shield.authc.support.ldap.ConnectionFactory; -import org.elasticsearch.shield.authc.support.ldap.AbstractLdapSslSocketFactory; import javax.naming.Context; -import javax.naming.NamingEnumeration; import javax.naming.NamingException; import javax.naming.directory.DirContext; import javax.naming.directory.InitialDirContext; @@ -86,24 +86,23 @@ public class ActiveDirectoryConnectionFactory extends ConnectionFactory { searchCtls.setReturningAttributes(Strings.EMPTY_ARRAY); searchCtls.setTimeLimit(timeoutMilliseconds); String searchFilter = "(&(objectClass=user)(userPrincipalName={0}))"; - NamingEnumeration results = ctx.search(userSearchDN, searchFilter, new Object[] { userPrincipal }, searchCtls); + try (ClosableNamingEnumeration results = new ClosableNamingEnumeration( + ctx.search(userSearchDN, searchFilter, new Object[] { userPrincipal }, searchCtls))) { - if (results.hasMore()) { - SearchResult entry = results.next(); - String name = entry.getNameInNamespace(); + if(results.hasMore()){ + SearchResult entry = results.next(); + String name = entry.getNameInNamespace(); - if (!results.hasMore()) { - return new ActiveDirectoryConnection(ctx, name, userSearchDN, timeoutMilliseconds); + if (!results.hasMore()) { + return new ActiveDirectoryConnection(ctx, name, userSearchDN, timeoutMilliseconds); + } + throw new ActiveDirectoryException("Search for user [" + userName + "] by principle name yielded multiple results"); } - results.close(); + ctx.close(); throw new ActiveDirectoryException("Search for user [" + userName + "] by principle name yielded multiple results"); } - results.close(); - ctx.close(); - throw new ActiveDirectoryException("Search for user [" + userName + "], search root [" + userSearchDN + "] yielded no results"); - - } catch (NamingException e) { + } catch (NamingException | LdapException e) { if (ctx != null) { try { ctx.close(); diff --git a/src/main/java/org/elasticsearch/shield/authc/ldap/LdapConnection.java b/src/main/java/org/elasticsearch/shield/authc/ldap/LdapConnection.java index 96e1b64e910..840823628b7 100644 --- a/src/main/java/org/elasticsearch/shield/authc/ldap/LdapConnection.java +++ b/src/main/java/org/elasticsearch/shield/authc/ldap/LdapConnection.java @@ -9,6 +9,7 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.logging.ESLogger; import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.shield.authc.support.ldap.AbstractLdapConnection; +import org.elasticsearch.shield.authc.support.ldap.ClosableNamingEnumeration; import javax.naming.NamingEnumeration; import javax.naming.NamingException; @@ -81,12 +82,12 @@ public class LdapConnection extends AbstractLdapConnection { "(|(objectclass=groupOfNames)(objectclass=groupOfUniqueNames)(objectclass=group)) " + "(|(uniqueMember={0})(member={0})))"; - try { - NamingEnumeration results = jndiContext.search(groupSearchDN, filter, new Object[] { userDn }, search); + try (ClosableNamingEnumeration results = new ClosableNamingEnumeration<>( + jndiContext.search(groupSearchDN, filter, new Object[] { userDn }, search))) { while (results.hasMoreElements()) { groups.add(results.next().getNameInNamespace()); } - } catch (NamingException e) { + } catch (NamingException | LdapException e ) { throw new LdapException("Could not search for an LDAP group for user [" + userDn + "]", e); } return groups; @@ -103,17 +104,19 @@ public class LdapConnection extends AbstractLdapConnection { List groupDns = new LinkedList<>(); try { Attributes results = jndiContext.getAttributes(userDn, new String[] { groupAttribute }); - for (NamingEnumeration ae = results.getAll(); ae.hasMore(); ) { - Attribute attr = (Attribute) ae.next(); - for (NamingEnumeration attrEnum = attr.getAll(); attrEnum.hasMore(); ) { - Object val = attrEnum.next(); - if (val instanceof String) { - String stringVal = (String) val; - groupDns.add(stringVal); + try (ClosableNamingEnumeration ae = new ClosableNamingEnumeration<>(results.getAll())) { + while (ae.hasMore()) { + Attribute attr = (Attribute) ae.next(); + for (NamingEnumeration attrEnum = attr.getAll(); attrEnum.hasMore(); ) { + Object val = attrEnum.next(); + if (val instanceof String) { + String stringVal = (String) val; + groupDns.add(stringVal); + } } } } - } catch (NamingException e) { + } catch (NamingException | LdapException e) { throw new LdapException("Could not look up group attributes for user [" + userDn + "]", e); } return groupDns; diff --git a/src/main/java/org/elasticsearch/shield/authc/support/ldap/ClosableNamingEnumeration.java b/src/main/java/org/elasticsearch/shield/authc/support/ldap/ClosableNamingEnumeration.java new file mode 100644 index 00000000000..80c6b31f279 --- /dev/null +++ b/src/main/java/org/elasticsearch/shield/authc/support/ldap/ClosableNamingEnumeration.java @@ -0,0 +1,52 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.shield.authc.support.ldap; + +import org.elasticsearch.shield.authc.ldap.LdapException; + +import javax.naming.NamingEnumeration; +import javax.naming.NamingException; +import java.io.Closeable; + +/** + * ClosableNamingEnumeration wraps a NamingEnumeration so it can be used in a try with resources block and auto-closed. + */ +public class ClosableNamingEnumeration implements Closeable, NamingEnumeration { + private final NamingEnumeration namingEnumeration; + + public ClosableNamingEnumeration(NamingEnumeration namingEnumeration) { + this.namingEnumeration = namingEnumeration; + } + + @Override + public T next() throws NamingException { + return namingEnumeration.next(); + } + + @Override + public boolean hasMore() throws NamingException { + return namingEnumeration.hasMore(); + } + + @Override + public void close() { + try { + namingEnumeration.close(); + } catch (NamingException e) { + throw new LdapException("Error occurred trying to close a naming enumeration", e); + } + } + + @Override + public boolean hasMoreElements() { + return namingEnumeration.hasMoreElements(); + } + + @Override + public T nextElement() { + return namingEnumeration.nextElement(); + } +}