From 79d4b1e20808f7e74bb8ca0b224d2b4df3a3c818 Mon Sep 17 00:00:00 2001 From: c-a-m Date: Fri, 16 Jan 2015 14:15:10 -0700 Subject: [PATCH] LDAP: Add configurable filters to LDAP group search and AD user search This lets the user configure custom filters for group searches in LDAP, and user searches in AD changed configuration in this commit: group_search.group_search_dn -> group_search.base_dn group_search.subtree_search -> group_search.subtree added for LDAP: group_search.filter group_search.user_attribute added for AD: user_search.base_dn user_search.filter user_search.subtree This also changes group_search.subtree to be true by default. This fixes elastic/elasticsearch#567 and fixes elastic/elasticsearch#553 Original commit: elastic/x-pack-elasticsearch@8a1246aefdcc089140f4c817834bc1ca211bc45d --- .../ActiveDirectoryConnection.java | 10 ++-- .../ActiveDirectoryConnectionFactory.java | 24 +++++--- .../shield/authc/ldap/LdapConnection.java | 57 ++++++++++++------- .../authc/ldap/LdapConnectionFactory.java | 20 +++++-- .../ActiveDirectoryFactoryTests.java | 31 ++++++++-- .../authc/ldap/LdapConnectionTests.java | 25 +++++++- .../shield/authc/ldap/OpenLdapTests.java | 17 ++++++ 7 files changed, 138 insertions(+), 46 deletions(-) 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 d269feff29d..0a452dd0893 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 @@ -63,12 +63,12 @@ public class ActiveDirectoryConnection extends AbstractLdapConnection { groupsSearchCtls.setTimeLimit(timeoutMilliseconds); ImmutableList.Builder groups = ImmutableList.builder(); - try (ClosableNamingEnumeration groupsAnswer = new ClosableNamingEnumeration( + try (ClosableNamingEnumeration groupsAnswer = new ClosableNamingEnumeration<>( jndiContext.search(groupSearchDN, groupsSearchFilter, groupsSearchCtls))) { //Loop through the search results while (groupsAnswer.hasMoreElements()) { - SearchResult sr = (SearchResult) groupsAnswer.next(); + SearchResult sr = groupsAnswer.next(); groups.add(sr.getNameInNamespace()); } } catch (NamingException | LdapException ne) { @@ -91,19 +91,19 @@ public class ActiveDirectoryConnection extends AbstractLdapConnection { String userSearchFilter = "(objectClass=user)"; String userReturnedAtts[] = { "tokenGroups" }; userSearchCtls.setReturningAttributes(userReturnedAtts); - try (ClosableNamingEnumeration userAnswer = new ClosableNamingEnumeration( + try (ClosableNamingEnumeration userAnswer = new ClosableNamingEnumeration<>( jndiContext.search(authenticatedUserDn(), userSearchFilter, userSearchCtls))) { //Loop through the search results while (userAnswer.hasMoreElements()) { - SearchResult sr = (SearchResult) userAnswer.next(); + SearchResult sr = userAnswer.next(); Attributes attrs = sr.getAttributes(); if (attrs != null) { try (ClosableNamingEnumeration ae = new ClosableNamingEnumeration<>(attrs.getAll())) { while (ae.hasMore() ) { - Attribute attr = (Attribute) ae.next(); + Attribute attr = ae.next(); for (NamingEnumeration e = attr.getAll(); e.hasMore(); ) { byte[] sid = (byte[]) e.next(); groupsSearchFilter.append("(objectSid="); 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 644522309b0..c7fe8671ab4 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 @@ -34,12 +34,16 @@ import java.util.Hashtable; public class ActiveDirectoryConnectionFactory extends ConnectionFactory { public static final String AD_DOMAIN_NAME_SETTING = "domain_name"; - public static final String AD_USER_SEARCH_BASEDN_SETTING = "user_search_dn"; + public static final String AD_USER_SEARCH_BASEDN_SETTING = "user_search.base_dn"; + public static final String AD_USER_SEARCH_FILTER_SETTING = "user_search.filter"; + public static final String AD_USER_SEARCH_SUBTREE_SETTING = "user_search.subtree"; private final ImmutableMap sharedLdapEnv; private final String userSearchDN; private final String domainName; + private final String userSearchFilter; private final int timeoutMilliseconds; + private final Boolean userSearchSubtree; @Inject public ActiveDirectoryConnectionFactory(RealmConfig config) { @@ -50,6 +54,8 @@ public class ActiveDirectoryConnectionFactory extends ConnectionFactory ldapEnv = new Hashtable<>(this.sharedLdapEnv); ldapEnv.put(Context.SECURITY_AUTHENTICATION, "simple"); ldapEnv.put(Context.SECURITY_PRINCIPAL, userPrincipal); @@ -84,12 +90,11 @@ public class ActiveDirectoryConnectionFactory extends ConnectionFactory results = new ClosableNamingEnumeration( - ctx.search(userSearchDN, searchFilter, new Object[] { userName, userPrincipal }, searchCtls))) { + try (ClosableNamingEnumeration results = new ClosableNamingEnumeration<>( + ctx.search(userSearchDN, userSearchFilter, new Object[] { userName }, searchCtls))) { if(results.hasMore()){ SearchResult entry = results.next(); @@ -98,11 +103,12 @@ public class ActiveDirectoryConnectionFactory extends ConnectionFactory groups() { - List groups = isFindGroupsByAttribute ? getGroupsFromUserAttrs(bindDn) : getGroupsFromSearch(bindDn); + List groups = isFindGroupsByAttribute ? getGroupsFromUserAttrs() : getGroupsFromSearch(); if (logger.isDebugEnabled()) { logger.debug("found groups [{}] for userDN [{}]", groups, this.bindDn); } @@ -63,29 +69,26 @@ public class LdapConnection extends AbstractLdapConnection { /** * Fetches the groups of a user by doing a search. This could be abstracted out into a strategy class or through * an inherited class (with groups as the template method). - * - * @param userDn user fully distinguished name to fetch group membership for * @return fully distinguished names of the roles */ - public List getGroupsFromSearch(String userDn) { + public List getGroupsFromSearch() { + String userIdentifier = userAttributeForGroupMembership == null ? bindDn : readUserAttribute(userAttributeForGroupMembership); + if (logger.isTraceEnabled()) { + logger.trace("user identifier for group lookup is [{}]", userIdentifier); + } List groups = new LinkedList<>(); SearchControls search = new SearchControls(); search.setReturningAttributes(Strings.EMPTY_ARRAY); search.setSearchScope(this.isGroupSubTreeSearch ? SearchControls.SUBTREE_SCOPE : SearchControls.ONELEVEL_SCOPE); search.setTimeLimit(timeoutMilliseconds); - //This could be made could be made configurable but it should cover all cases - String filter = "(&" + - "(|(objectclass=groupOfNames)(objectclass=groupOfUniqueNames)(objectclass=group)) " + - "(|(uniqueMember={0})(member={0})))"; - try (ClosableNamingEnumeration results = new ClosableNamingEnumeration<>( - jndiContext.search(groupSearchDN, filter, new Object[] { userDn }, search))) { + jndiContext.search(groupSearchDN, groupSearchFilter, new Object[] {userIdentifier}, search))) { while (results.hasMoreElements()) { groups.add(results.next().getNameInNamespace()); } } catch (NamingException | LdapException e ) { - throw new LdapException("could not search for an LDAP group for user [" + userDn + "]", e); + throw new LdapException("could not search for an LDAP group", bindDn, e); } return groups; } @@ -94,16 +97,15 @@ public class LdapConnection extends AbstractLdapConnection { * Fetches the groups from the user attributes (if supported). This method could later be abstracted out * into a strategy class * - * @param userDn User fully distinguished name to fetch group membership from * @return list of groups the user is a member of. */ - public List getGroupsFromUserAttrs(String userDn) { + public List getGroupsFromUserAttrs() { List groupDns = new LinkedList<>(); try { - Attributes results = jndiContext.getAttributes(userDn, new String[] { groupAttribute }); + Attributes results = jndiContext.getAttributes(bindDn, new String[] { groupAttribute }); try (ClosableNamingEnumeration ae = new ClosableNamingEnumeration<>(results.getAll())) { while (ae.hasMore()) { - Attribute attr = (Attribute) ae.next(); + Attribute attr = ae.next(); for (NamingEnumeration attrEnum = attr.getAll(); attrEnum.hasMore(); ) { Object val = attrEnum.next(); if (val instanceof String) { @@ -114,8 +116,23 @@ public class LdapConnection extends AbstractLdapConnection { } } } catch (NamingException | LdapException e) { - throw new LdapException("could not look up group attributes for user [" + userDn + "]", e); + throw new LdapException("could not look up group attributes for user", bindDn, e); } return groupDns; } + + String readUserAttribute(String userAttribute) { + try { + Attributes results = jndiContext.getAttributes(bindDn, new String[]{userAttribute}); + Attribute attribute = results.get(userAttribute); + if (results.size() == 0) { + throw new LdapException("No results returned for attribute [" + userAttribute + "]", bindDn); + } + return (String) attribute.get(); + } catch (NamingException e) { + throw new LdapException("Could not look attribute [" + userAttribute + "]", bindDn, e); + } catch (ClassCastException e) { + throw new LdapException("Returned ldap attribute [" + userAttribute + "] is not of type String", bindDn, e); + } + } } diff --git a/src/main/java/org/elasticsearch/shield/authc/ldap/LdapConnectionFactory.java b/src/main/java/org/elasticsearch/shield/authc/ldap/LdapConnectionFactory.java index 3b7dbf63293..5c16f65df20 100644 --- a/src/main/java/org/elasticsearch/shield/authc/ldap/LdapConnectionFactory.java +++ b/src/main/java/org/elasticsearch/shield/authc/ldap/LdapConnectionFactory.java @@ -32,8 +32,14 @@ import java.util.Hashtable; public class LdapConnectionFactory extends ConnectionFactory { public static final String USER_DN_TEMPLATES_SETTING = "user_dn_templates"; - public static final String GROUP_SEARCH_SUBTREE_SETTING = "group_search.subtree_search"; - public static final String GROUP_SEARCH_BASEDN_SETTING = "group_search.group_search_dn"; + public static final String GROUP_SEARCH_SUBTREE_SETTING = "group_search.subtree"; + public static final String GROUP_SEARCH_BASEDN_SETTING = "group_search.base_dn"; + public static final String GROUP_SEARCH_FILTER_SETTING = "group_search.filter"; + public static final String GROUP_SEARCH_USER_ATTRIBUTE_SETTING = "group_search.user_attribute"; + + private static final String GROUP_SEARCH_DEFAULT_FILTER = "(&" + + "(|(objectclass=groupOfNames)(objectclass=groupOfUniqueNames)(objectclass=group))" + + "(|(uniqueMember={0})(member={0})))"; private final ImmutableMap sharedLdapEnv; private final String[] userDnTemplates; @@ -41,6 +47,8 @@ public class LdapConnectionFactory extends ConnectionFactory { protected final boolean groupSubTreeSearch; protected final boolean findGroupsByAttribute; private final int timeoutMilliseconds; + private final String groupFilter; + private final String groupSearchUserAttribute; @Inject() public LdapConnectionFactory(RealmConfig config) { @@ -69,7 +77,10 @@ public class LdapConnectionFactory extends ConnectionFactory { sharedLdapEnv = builder.build(); groupSearchDN = settings.get(GROUP_SEARCH_BASEDN_SETTING); findGroupsByAttribute = groupSearchDN == null; - groupSubTreeSearch = settings.getAsBoolean(GROUP_SEARCH_SUBTREE_SETTING, false); + groupSubTreeSearch = settings.getAsBoolean(GROUP_SEARCH_SUBTREE_SETTING, true); + groupFilter = settings.get(GROUP_SEARCH_FILTER_SETTING, GROUP_SEARCH_DEFAULT_FILTER); + groupSearchUserAttribute = settings.get(GROUP_SEARCH_FILTER_SETTING) == null ? + null : settings.get(GROUP_SEARCH_USER_ATTRIBUTE_SETTING); //if filter isn't set we don't want to change from using the DN } /** @@ -93,7 +104,8 @@ public class LdapConnectionFactory extends ConnectionFactory { DirContext ctx = new InitialDirContext(ldapEnv); //return the first good connection - return new LdapConnection(connectionLogger, ctx, dn, findGroupsByAttribute, groupSubTreeSearch, groupSearchDN, timeoutMilliseconds); + return new LdapConnection(connectionLogger, ctx, dn, timeoutMilliseconds, findGroupsByAttribute, groupSubTreeSearch, + groupFilter, groupSearchDN, groupSearchUserAttribute); } catch (NamingException e) { logger.warn("failed LDAP authentication with user template [{}] and DN [{}]", e, template, dn); diff --git a/src/test/java/org/elasticsearch/shield/authc/active_directory/ActiveDirectoryFactoryTests.java b/src/test/java/org/elasticsearch/shield/authc/active_directory/ActiveDirectoryFactoryTests.java index f8c0d19458f..17f0c14835c 100644 --- a/src/test/java/org/elasticsearch/shield/authc/active_directory/ActiveDirectoryFactoryTests.java +++ b/src/test/java/org/elasticsearch/shield/authc/active_directory/ActiveDirectoryFactoryTests.java @@ -109,7 +109,7 @@ public class ActiveDirectoryFactoryTests extends ElasticsearchTestCase { } @Test @SuppressWarnings("unchecked") - public void testAdAuth_specificUserSearch() { + public void testAuthenticate_specificUserSearch() { Settings settings = buildAdSettings(AD_LDAP_URL, AD_DOMAIN, "CN=Users,DC=ad,DC=test,DC=elasticsearch,DC=com", false); RealmConfig config = new RealmConfig("ad-test", settings); ActiveDirectoryConnectionFactory connectionFactory = new ActiveDirectoryConnectionFactory(config); @@ -130,7 +130,7 @@ public class ActiveDirectoryFactoryTests extends ElasticsearchTestCase { } @Test @SuppressWarnings("unchecked") - public void testAdUpnLogin() { + public void testAuthenticate_UserPrincipalName() { Settings settings = buildAdSettings(AD_LDAP_URL, AD_DOMAIN, "CN=Users,DC=ad,DC=test,DC=elasticsearch,DC=com", false); RealmConfig config = new RealmConfig("ad-test", settings); ActiveDirectoryConnectionFactory connectionFactory = new ActiveDirectoryConnectionFactory(config); @@ -156,7 +156,26 @@ public class ActiveDirectoryFactoryTests extends ElasticsearchTestCase { } @Test @SuppressWarnings("unchecked") - public void testAD_standardLdapConnection(){ + public void testCustomUserFilter() { + Settings settings = ImmutableSettings.builder() + .put(buildAdSettings(AD_LDAP_URL, AD_DOMAIN, "CN=Users,DC=ad,DC=test,DC=elasticsearch,DC=com", false)) + .put(ActiveDirectoryConnectionFactory.AD_USER_SEARCH_FILTER_SETTING, "(&(objectclass=user)(userPrincipalName={0}@ad.test.elasticsearch.com))") + .build(); + RealmConfig config = new RealmConfig("ad-test", settings); + ActiveDirectoryConnectionFactory connectionFactory = new ActiveDirectoryConnectionFactory(config); + + //Login with the UserPrincipalName + try (AbstractLdapConnection ldap = connectionFactory.open("erik.selvig", SecuredStringTests.build(PASSWORD))) { + List groups = ldap.groups(); + assertThat(groups, containsInAnyOrder( + containsString("Geniuses"), + containsString("Domain Users"))); + } + } + + + @Test @SuppressWarnings("unchecked") + public void testStandardLdapConnection(){ String groupSearchBase = "DC=ad,DC=test,DC=elasticsearch,DC=com"; String userTemplate = "CN={0},CN=Users,DC=ad,DC=test,DC=elasticsearch,DC=com"; Settings settings = LdapTest.buildLdapSettings(AD_LDAP_URL, userTemplate, groupSearchBase, true, false); @@ -165,8 +184,8 @@ public class ActiveDirectoryFactoryTests extends ElasticsearchTestCase { String user = "Bruce Banner"; try (LdapConnection ldap = connectionFactory.open(user, SecuredStringTests.build(PASSWORD))) { - List groups = ldap.getGroupsFromUserAttrs(ldap.authenticatedUserDn()); - List groups2 = ldap.getGroupsFromSearch(ldap.authenticatedUserDn()); + List groups = ldap.getGroupsFromUserAttrs(); + List groups2 = ldap.getGroupsFromSearch(); assertThat(groups, containsInAnyOrder( containsString("Avengers"), @@ -194,7 +213,7 @@ public class ActiveDirectoryFactoryTests extends ElasticsearchTestCase { } @Test(expected = LdapException.class) - public void testADStandardLdapHostnameVerification(){ + public void testStandardLdapHostnameVerification(){ String groupSearchBase = "DC=ad,DC=test,DC=elasticsearch,DC=com"; String userTemplate = "CN={0},CN=Users,DC=ad,DC=test,DC=elasticsearch,DC=com"; Settings settings = LdapTest.buildLdapSettings(AD_LDAP_URL, userTemplate, groupSearchBase, true); diff --git a/src/test/java/org/elasticsearch/shield/authc/ldap/LdapConnectionTests.java b/src/test/java/org/elasticsearch/shield/authc/ldap/LdapConnectionTests.java index 684b03d0ae0..c834ed78cf7 100644 --- a/src/test/java/org/elasticsearch/shield/authc/ldap/LdapConnectionTests.java +++ b/src/test/java/org/elasticsearch/shield/authc/ldap/LdapConnectionTests.java @@ -114,7 +114,7 @@ public class LdapConnectionTests extends LdapTest { SecuredString userPass = SecuredStringTests.build("pass"); try (LdapConnection ldap = ldapFac.open(user, userPass)) { - List groups = ldap.getGroupsFromSearch(ldap.authenticatedUserDn()); + List groups = ldap.getGroupsFromSearch(); assertThat(groups, contains("cn=HMS Lydia,ou=crews,ou=groups,o=sevenSeas")); } } @@ -129,8 +129,29 @@ public class LdapConnectionTests extends LdapTest { String user = "Horatio Hornblower"; try (LdapConnection ldap = ldapFac.open(user, SecuredStringTests.build("pass"))) { - List groups = ldap.getGroupsFromSearch(ldap.authenticatedUserDn()); + List groups = ldap.getGroupsFromSearch(); assertThat(groups, contains("cn=HMS Lydia,ou=crews,ou=groups,o=sevenSeas")); } } + + @Test + public void testUserAttributeLookup() { + String groupSearchBase = "ou=crews,ou=groups,o=sevenSeas"; + String userTemplate = "cn={0},ou=people,o=sevenSeas"; + RealmConfig config = new RealmConfig("ldap_realm", buildLdapSettings(ldapUrl(), userTemplate, groupSearchBase, false)); + LdapConnectionFactory ldapFac = new LdapConnectionFactory(config); + + + String user = "Horatio Hornblower"; + try (LdapConnection ldap = ldapFac.open(user, SecuredStringTests.build("pass"))) { + assertThat(ldap.readUserAttribute("mail"), is("hhornblo@royalnavy.mod.uk")); + assertThat(ldap.readUserAttribute("uid"), is("hhornblo")); + try { + ldap.readUserAttribute("nonexistentAttribute"); + fail("reading a non existent attribute should throw an LDAPException"); + } catch (LdapException e) { + assertThat(e.getMessage(), containsString("No results")); + } + } + } } diff --git a/src/test/java/org/elasticsearch/shield/authc/ldap/OpenLdapTests.java b/src/test/java/org/elasticsearch/shield/authc/ldap/OpenLdapTests.java index 5100b6371d3..50e3786be13 100644 --- a/src/test/java/org/elasticsearch/shield/authc/ldap/OpenLdapTests.java +++ b/src/test/java/org/elasticsearch/shield/authc/ldap/OpenLdapTests.java @@ -71,6 +71,23 @@ public class OpenLdapTests extends ElasticsearchTestCase { } } + @Test + public void testCustomFilter() { + String groupSearchBase = "ou=people,dc=oldap,dc=test,dc=elasticsearch,dc=com"; + String userTemplate = "uid={0},ou=people,dc=oldap,dc=test,dc=elasticsearch,dc=com"; + Settings settings = ImmutableSettings.builder() + .put(LdapConnectionTests.buildLdapSettings(OPEN_LDAP_URL, userTemplate,groupSearchBase, true, false)) + .put(LdapConnectionFactory.GROUP_SEARCH_FILTER_SETTING, "(&(objectclass=posixGroup)(memberUID={0}))") + .put(LdapConnectionFactory.GROUP_SEARCH_USER_ATTRIBUTE_SETTING, "uid") + .build(); + RealmConfig config = new RealmConfig("oldap-test", settings); + LdapConnectionFactory connectionFactory = new LdapConnectionFactory(config); + + try (LdapConnection ldap = connectionFactory.open("selvig", SecuredStringTests.build(PASSWORD))){ + assertThat(ldap.groups(), hasItem(containsString("Geniuses"))); + } + } + @Test @LuceneTestCase.AwaitsFix(bugUrl = "https://github.com/elasticsearch/elasticsearch-shield/issues/499") public void testTcpTimeout() { String groupSearchBase = "ou=people,dc=oldap,dc=test,dc=elasticsearch,dc=com";