Skip to content

Update LDAP authority and role handling to use LdapClient #17035

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Update LDAP authority and role handling to use LdapClient #17035

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .../security/ldap/userdetails/DefaultLdapAuthoritiesPopulatorGetGrantedAuthoritiesTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,11 @@ public class DefaultLdapAuthoritiesPopulatorGetGrantedAuthoritiesTests {
@BeforeEach
public void setUp() {
this.populator = new DefaultLdapAuthoritiesPopulator(this.contextSource, "ou=groups");
this.populator.setIgnorePartialResultException(false);
// this.populator.setIgnorePartialResultException(false);
}

@Test
// fix me this is broken with the LdapClient
public void groupSearchDoesNotAllowNullRoles() {
this.populator.setRolePrefix("ROLE_");
this.populator.setGroupRoleAttribute("ou");
Expand Down
23 changes: 17 additions & 6 deletions ...a/org/springframework/security/ldap/userdetails/DefaultLdapAuthoritiesPopulatorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.springframework.security.ldap.userdetails;

import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;

Expand All @@ -36,6 +37,9 @@
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit.jupiter.SpringExtension;

import javax.naming.Name;
import javax.naming.NamingException;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;

Expand All @@ -56,13 +60,13 @@ public class DefaultLdapAuthoritiesPopulatorTests {
@BeforeEach
public void setUp() {
this.populator = new DefaultLdapAuthoritiesPopulator(this.contextSource, "ou=groups");
this.populator.setIgnorePartialResultException(false);
// this.populator.setIgnorePartialResultException(false);
}

@Test
public void defaultRoleIsAssignedWhenSet() {
this.populator.setDefaultRole("ROLE_USER");
assertThat(this.populator.getContextSource()).isSameAs(this.contextSource);
// assertThat(this.populator.getContextSource()).isSameAs(this.contextSource);

DirContextAdapter ctx = new DirContextAdapter(new DistinguishedName("cn=notfound"));

Expand Down Expand Up @@ -185,10 +189,17 @@ public void userDnWithEscapedCharacterParameterReturnsExpectedRoles() {

@Test
public void customAuthoritiesMappingFunction() {
this.populator.setAuthorityMapper((record) -> {
String dn = record.get(SpringSecurityLdapTemplate.DN_KEY).get(0);
String role = record.get(this.populator.getGroupRoleAttribute()).get(0);
return new LdapAuthority(role, dn);
this.populator.setAuthorityMapper((entry) -> {
Name dn;
String role;
try {
dn = entry.getDn();
role = entry.getAttributes().get(this.populator.getGroupRoleAttribute()).get().toString();
}
catch (NamingException e) {
throw new RuntimeException(e);
}
return Collections.singleton(new LdapAuthority(role, dn));
});

DirContextAdapter ctx = new DirContextAdapter(
Expand Down
51 changes: 28 additions & 23 deletions ...va/org/springframework/security/ldap/userdetails/NestedLdapAuthoritiesPopulatorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@

package org.springframework.security.ldap.userdetails;

import java.util.Arrays;
import java.util.Collection;
import java.util.HashSet;
import java.util.*;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand All @@ -32,6 +30,10 @@
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit.jupiter.SpringExtension;

import javax.naming.InvalidNameException;
import javax.naming.NamingException;
import javax.naming.ldap.LdapName;

import static org.assertj.core.api.Assertions.assertThat;

/**
Expand Down Expand Up @@ -60,24 +62,25 @@ public class NestedLdapAuthoritiesPopulatorTests {
private LdapAuthority circularJavaDevelopers;

@BeforeEach
public void setUp() {
public void setUp() throws InvalidNameException {
this.populator = new NestedLdapAuthoritiesPopulator(this.contextSource, "ou=jdeveloper");
this.populator.setGroupSearchFilter("(member={0})");
this.populator.setIgnorePartialResultException(false);
// this.populator.setIgnorePartialResultException(false);
this.populator.setRolePrefix("");
this.populator.setSearchSubtree(true);
this.populator.setConvertToUpperCase(false);
this.jDevelopers = new LdapAuthority("j-developers", "cn=j-developers,ou=jdeveloper,dc=springframework,dc=org");
this.jDevelopers = new LdapAuthority("j-developers",
new LdapName("cn=j-developers,ou=jdeveloper,dc=springframework,dc=org"));
this.javaDevelopers = new LdapAuthority("java-developers",
"cn=java-developers,ou=jdeveloper,dc=springframework,dc=org");
new LdapName("cn=java-developers,ou=jdeveloper,dc=springframework,dc=org"));
this.groovyDevelopers = new LdapAuthority("groovy-developers",
"cn=groovy-developers,ou=jdeveloper,dc=springframework,dc=org");
new LdapName("cn=groovy-developers,ou=jdeveloper,dc=springframework,dc=org"));
this.scalaDevelopers = new LdapAuthority("scala-developers",
"cn=scala-developers,ou=jdeveloper,dc=springframework,dc=org");
new LdapName("cn=scala-developers,ou=jdeveloper,dc=springframework,dc=org"));
this.closureDevelopers = new LdapAuthority("closure-developers",
"cn=closure-developers,ou=jdeveloper,dc=springframework,dc=org");
new LdapName("cn=closure-developers,ou=jdeveloper,dc=springframework,dc=org"));
this.circularJavaDevelopers = new LdapAuthority("circular-java-developers",
"cn=circular-java-developers,ou=jdeveloper,dc=springframework,dc=org");
new LdapName("cn=circular-java-developers,ou=jdeveloper,dc=springframework,dc=org"));
}

@Test
Expand All @@ -95,6 +98,9 @@ public void testJavaDudeJDevelopersAuthorities() {
Collection<GrantedAuthority> authorities = this.populator.getGrantedAuthorities(ctx, "javadude");
assertThat(authorities).hasSize(4);
assertThat(authorities).contains(this.javaDevelopers);
assertThat(authorities).contains(this.circularJavaDevelopers);
assertThat(authorities).contains(this.groovyDevelopers);
assertThat(authorities).contains(this.jDevelopers);
}

@Test
Expand All @@ -103,21 +109,21 @@ public void testScalaDudeJDevelopersAuthoritiesWithSearchLimit() {
DirContextAdapter ctx = new DirContextAdapter("uid=scaladude,ou=people,dc=springframework,dc=org");
Collection<GrantedAuthority> authorities = this.populator.getGrantedAuthorities(ctx, "scaladude");
assertThat(authorities).hasSize(1);
assertThat(authorities).isEqualTo(Arrays.asList(this.scalaDevelopers));
assertThat(authorities).isEqualTo(Collections.singletonList(this.scalaDevelopers));
}

@Test
public void testGroovyDudeJDevelopersAuthorities() {
DirContextAdapter ctx = new DirContextAdapter("uid=groovydude,ou=people,dc=springframework,dc=org");
DirContextAdapter ctx = new DirContextAdapter("uid=groovydude,ou=people");
Collection<GrantedAuthority> authorities = this.populator.getGrantedAuthorities(ctx, "groovydude");
assertThat(authorities).hasSize(4);
assertThat(authorities).isEqualTo(Arrays.asList(this.javaDevelopers, this.circularJavaDevelopers,
this.groovyDevelopers, this.jDevelopers));
}

@Test
public void testClosureDudeJDevelopersWithMembershipAsAttributeValues() {
this.populator.setAttributeNames(new HashSet(Arrays.asList("member")));
public void testClosureDudeJDevelopersWithMembershipAsAttributeValues() throws NamingException {
this.populator.setAttributeNames(new HashSet<>(List.of("member")));

DirContextAdapter ctx = new DirContextAdapter("uid=closuredude,ou=people,dc=springframework,dc=org");
Collection<GrantedAuthority> authorities = this.populator.getGrantedAuthorities(ctx, "closuredude");
Expand All @@ -128,19 +134,18 @@ public void testClosureDudeJDevelopersWithMembershipAsAttributeValues() {
LdapAuthority[] ldapAuthorities = authorities.toArray(new LdapAuthority[0]);
assertThat(ldapAuthorities).hasSize(5);
// groovy-developers group
assertThat(ldapAuthorities[0].getAttributes()).containsKey("member");
assertThat(ldapAuthorities[0].getAttributes().get("member")).isNotNull();
assertThat(ldapAuthorities[0].getAttributes().get("member")).hasSize(3);
assertThat(ldapAuthorities[0].getFirstAttributeValue("member"))
.isEqualTo("cn=groovy-developers,ou=jdeveloper,dc=springframework,dc=org");
assertThat(ldapAuthorities[0].getAttributes().get("member")).isNotNull();
assertThat(ldapAuthorities[0].getAttributes().get("member").size()).isEqualTo(3);
assertThat(ldapAuthorities[0].getFirstAttributeValue("member")).isEqualTo("cn=groovy-developers,ou=jdeveloper");

// java group
assertThat(ldapAuthorities[1].getAttributes()).containsKey("member");
assertThat(ldapAuthorities[1].getAttributes().get("member")).isNotNull();
assertThat(ldapAuthorities[1].getAttributes().get("member")).hasSize(3);
assertThat(ldapAuthorities[1].getAttributes().get("member")).isNotNull();
assertThat(ldapAuthorities[1].getAttributes().get("member").size()).isEqualTo(3);
assertThat(this.groovyDevelopers.getDn()).isEqualTo(ldapAuthorities[1].getFirstAttributeValue("member"));
assertThat(ldapAuthorities[2].getAttributes().get("member"))
.contains("uid=closuredude,ou=people,dc=springframework,dc=org");
assertThat(ldapAuthorities[2].getAttributes().get("member").get().toString())
.contains("uid=closuredude,ou=people");

// test non existent attribute
assertThat(ldapAuthorities[2].getFirstAttributeValue("test")).isNull();
Expand Down
121 changes: 51 additions & 70 deletions ...n/java/org/springframework/security/ldap/userdetails/DefaultLdapAuthoritiesPopulator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,29 +16,26 @@

package org.springframework.security.ldap.userdetails;

import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.function.Function;

import javax.naming.directory.SearchControls;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;

import org.springframework.LdapDataEntry;
import org.springframework.core.log.LogMessage;
import org.springframework.ldap.core.ContextSource;
import org.springframework.ldap.core.DirContextOperations;
import org.springframework.ldap.core.LdapTemplate;
import org.springframework.ldap.core.LdapClient;
import org.springframework.ldap.query.LdapQuery;
import org.springframework.ldap.query.LdapQueryBuilder;
import org.springframework.ldap.query.SearchScope;
import org.springframework.ldap.support.LdapUtils;
import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.core.authority.SimpleGrantedAuthority;
import org.springframework.security.ldap.SpringSecurityLdapTemplate;
import org.springframework.util.Assert;
import org.springframework.util.CollectionUtils;

import javax.naming.NamingException;
import javax.naming.directory.Attribute;
import javax.naming.directory.Attributes;
import java.util.*;
import java.util.function.Function;

/**
* The default strategy for obtaining user role information from the directory.
Expand Down Expand Up @@ -112,15 +109,15 @@ public class DefaultLdapAuthoritiesPopulator implements LdapAuthoritiesPopulator
private GrantedAuthority defaultRole;

/**
* Template that will be used for searching
* LDAP client that will be used for searching
*/
private final SpringSecurityLdapTemplate ldapTemplate;
private final LdapClient ldapClient;

/**
* Controls used to determine whether group searches should be performed over the full
* sub-tree from the base DN. Modified by searchSubTree property
*/
private final SearchControls searchControls = new SearchControls();
private SearchScope searchScope = SearchScope.ONELEVEL;

/**
* The ID of the attribute which contains the role name for a group
Expand Down Expand Up @@ -150,7 +147,7 @@ public class DefaultLdapAuthoritiesPopulator implements LdapAuthoritiesPopulator
/**
* The mapping function to be used to populate authorities.
*/
private Function<Map<String, List<String>>, GrantedAuthority> authorityMapper;
private Function<LdapDataEntry, Set<? extends GrantedAuthority>> authorityMapper;

/**
* Constructor for group search scenarios. <tt>userRoleAttributes</tt> may still be
Expand All @@ -161,28 +158,34 @@ public class DefaultLdapAuthoritiesPopulator implements LdapAuthoritiesPopulator
*/
public DefaultLdapAuthoritiesPopulator(ContextSource contextSource, String groupSearchBase) {
Assert.notNull(contextSource, "contextSource must not be null");
this.ldapTemplate = new SpringSecurityLdapTemplate(contextSource);
getLdapTemplate().setSearchControls(getSearchControls());
this.ldapClient = LdapClient.create(contextSource);
this.groupSearchBase = groupSearchBase;
if (groupSearchBase == null) {
logger.info("Will not perform group search since groupSearchBase is null.");
}
else if (groupSearchBase.isEmpty()) {
logger.info("Will perform group search from the context source base since groupSearchBase is empty.");
}
this.authorityMapper = (record) -> {
List<String> roles = record.get(this.groupRoleAttribute);
if (CollectionUtils.isEmpty(roles)) {
return null;
this.authorityMapper = (entry) -> {
try {
Attributes record = entry.getAttributes();
Set<LdapAuthority> authorities = new HashSet<>();
Attribute userRoles = record.get(this.groupRoleAttribute);
if (userRoles != null)
userRoles.getAll().asIterator().forEachRemaining(role -> {
if (role instanceof String s) {
if (this.convertToUpperCase) {
s = s.toUpperCase(Locale.ROOT);
}
authorities.add(new LdapAuthority(this.rolePrefix + s, entry.getDn()));
}
});

return authorities;
}
String role = roles.get(0);
if (role == null) {
return null;
catch (NamingException e) {
throw LdapUtils.convertLdapException(e);
}
if (this.convertToUpperCase) {
role = role.toUpperCase(Locale.ROOT);
}
return new SimpleGrantedAuthority(this.rolePrefix + role);
};
}

Expand Down Expand Up @@ -229,21 +232,19 @@ public Set<GrantedAuthority> getGroupMembershipRoles(String userDn, String usern
Set<GrantedAuthority> authorities = new HashSet<>();
logger.trace(LogMessage.of(() -> "Searching for roles for user " + username + " with DN " + userDn
+ " and filter " + this.groupSearchFilter + " in search base " + getGroupSearchBase()));
Set<Map<String, List<String>>> userRoles = getLdapTemplate().searchForMultipleAttributeValues(
getGroupSearchBase(), this.groupSearchFilter, new String[] { userDn, username },
new String[] { this.groupRoleAttribute });
logger.debug(LogMessage.of(() -> "Found roles from search " + userRoles));
for (Map<String, List<String>> role : userRoles) {
GrantedAuthority authority = this.authorityMapper.apply(role);
if (authority != null) {
authorities.add(authority);
}
}
return authorities;
}

protected ContextSource getContextSource() {
return getLdapTemplate().getContextSource();
LdapQuery query = LdapQueryBuilder.query()
.searchScope(searchScope)
.base(getGroupSearchBase())
.attributes(getGroupRoleAttribute())
.filter(getGroupSearchFilter(), userDn, username);

getLdapClient().search().query(query).toEntryStream().forEach(entry -> {
logger.debug(LogMessage.of(() -> "Found roles from search " + entry));

authorities.addAll(this.authorityMapper.apply(entry));
});
return authorities;
}

protected String getGroupSearchBase() {
Expand Down Expand Up @@ -292,26 +293,15 @@ public void setRolePrefix(String rolePrefix) {
* <tt>groupSearchBase</tt>.
*/
public void setSearchSubtree(boolean searchSubtree) {
int searchScope = searchSubtree ? SearchControls.SUBTREE_SCOPE : SearchControls.ONELEVEL_SCOPE;
this.searchControls.setSearchScope(searchScope);
}

/**
* Sets the corresponding property on the underlying template, avoiding specific
* issues with Active Directory.
*
* @see LdapTemplate#setIgnoreNameNotFoundException(boolean)
*/
public void setIgnorePartialResultException(boolean ignore) {
getLdapTemplate().setIgnorePartialResultException(ignore);
this.searchScope = searchSubtree ? SearchScope.SUBTREE : SearchScope.ONELEVEL;
}

/**
* Sets the mapping function which will be used to create instances of
* {@link GrantedAuthority} given the context record.
* @param authorityMapper the mapping function
*/
public void setAuthorityMapper(Function<Map<String, List<String>>, GrantedAuthority> authorityMapper) {
public void setAuthorityMapper(Function<LdapDataEntry, Set<? extends GrantedAuthority>> authorityMapper) {
Assert.notNull(authorityMapper, "authorityMapper must not be null");
this.authorityMapper = authorityMapper;
}
Expand All @@ -322,8 +312,8 @@ public void setAuthorityMapper(Function<Map<String, List<String>>, GrantedAuthor
* @return the LDAP template
* @see org.springframework.security.ldap.SpringSecurityLdapTemplate
*/
protected SpringSecurityLdapTemplate getLdapTemplate() {
return this.ldapTemplate;
protected LdapClient getLdapClient() {
return this.ldapClient;
}

/**
Expand Down Expand Up @@ -366,13 +356,4 @@ protected final boolean isConvertToUpperCase() {
return this.convertToUpperCase;
}

/**
* Returns the search controls Method available so that classes extending this can
* override the search controls used
* @return the search controls
*/
private SearchControls getSearchControls() {
return this.searchControls;
}

}
Loading